5 ms·

How so?

foepys 5y ago
It has no escape logic. Okay for scripts, as GP stated, very bad for production code.
Jon_Lowtek 5y ago
code lacks context sensitive escaping api_path = base_url + urllib.parse.urlencode({ 'action': action, 'format': letThisBeVariable, ... 'gscoord': str(latitude.value) + '|' + str(longitude.value) }) see: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode https://docs.python.org/3/library/urllib.parse.html#urllib.p... Mantra: when inserting data into a context (like an url) escape the data for that context.
firebaze 5y ago
Concatenating strings for example. As shown, it's the query string equivalent of sql injection. Use something like URLBuilder, or URIParams, or whatever your platform supports. Don't use string concatenation ever, if at all possible, and if not possible (wtf?), then at least escape strings.
dastbe 5y ago
the "nice" way of doing this would would be to create a list of your stringified arguments, mapped urlencoding over them, and then join them with the parameter separator. this ends up being resilient to someone adding something that ends up being incorrect, and makes explicit in the code what you're trying to do.
cmckn 5y ago
I usually try to avoid working with URLs as bare strings like this, both for readability and correctness (URL encoding is tricky). With ‘requests’ you can do something like pass a dictionary of your query params and it takes care of forming the actual request URL. https://docs.python-requests.org/en/latest/user/quickstart/#passing-parameters-in-urls https://docs.python-requests.org/en/latest/user/quickstart/#...
deleted 5y ago
[deleted]
deleted 5y ago
[deleted]
gen220 5y ago
It's much safer (i.e. fewer degrees of freedom for bugs to appear) to use f-strings in situations like this one. One correlated but ancillary benefit, is that there are fewer variables to simulate the state for in your brain, while you're reading the code. You don't have to wonder if a variable is going to change on you, in-between when it is initialized and when it is used. It's safer still to use a library (e.g. urllib3) that does encoding for you (allowing you to omit magic strings like `"%7C"` from the logic of this function alltogther). Like GP said, very handy for one-off scripts or areas of your codebase where quality is "less important". I may be pedantic, but I wouldn't give this a pass on code review.
deleted 5y ago
[deleted]

How so?

It has no escape logic. Okay for scripts, as GP stated, very bad for production code.

code lacks context sensitive escaping api_path = base_url + urllib.parse.urlencode({ 'action': action, 'format': letThisBeVariable, ... 'gscoord': str(latitude.value) + '|' + str(longitude.value) }) see: https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode https://docs.python.org/3/library/urllib.parse.html#urllib.p... Mantra: when inserting data into a context (like an url) escape the data for that context.

firebaze 5y ago

Concatenating strings for example. As shown, it's the query string equivalent of sql injection. Use something like URLBuilder, or URIParams, or whatever your platform supports. Don't use string concatenation ever, if at all possible, and if not possible (wtf?), then at least escape strings.

dastbe 5y ago

the "nice" way of doing this would would be to create a list of your stringified arguments, mapped urlencoding over them, and then join them with the parameter separator. this ends up being resilient to someone adding something that ends up being incorrect, and makes explicit in the code what you're trying to do.

cmckn 5y ago

I usually try to avoid working with URLs as bare strings like this, both for readability and correctness (URL encoding is tricky). With ‘requests’ you can do something like pass a dictionary of your query params and it takes care of forming the actual request URL. https://docs.python-requests.org/en/latest/user/quickstart/#passing-parameters-in-urls https://docs.python-requests.org/en/latest/user/quickstart/#...

deleted 5y ago

[deleted]

deleted 5y ago

[deleted]

gen220 5y ago

It's much safer (i.e. fewer degrees of freedom for bugs to appear) to use f-strings in situations like this one. One correlated but ancillary benefit, is that there are fewer variables to simulate the state for in your brain, while you're reading the code. You don't have to wonder if a variable is going to change on you, in-between when it is initialized and when it is used. It's safer still to use a library (e.g. urllib3) that does encoding for you (allowing you to omit magic strings like `"%7C"` from the logic of this function alltogther). Like GP said, very handy for one-off scripts or areas of your codebase where quality is "less important". I may be pedantic, but I wouldn't give this a pass on code review.

deleted 5y ago

[deleted]