Hacker News new | past | comments | ask | show | jobs | submit login

How so?



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.


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/#...


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.


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.p...

Mantra: when inserting data into a context (like an url) escape the data for that context.


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


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.




Consider applying for YC's Spring batch! Applications are open till Feb 11.

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: