RFC-2396-encode URLs

[Gallaecio]

No description provided.

[Gallaecio]

@kmike brought up that it is not RFC-3986, but java.net.URI’s flavor of RFC-2396, that we need to support.

After looking at the standards, and Java’s deviations, I think for our purposes updating the lists of characters to escape in different URL parts to those of the older standard is enough, and Java’s deviations do not seem to affect those, other than they make it clear that (escaped) UTF-8 characters are supported.

[codecov-commenter]

Codecov Report

Merging #37 (a3703b4) into main (b2edeac) will increase coverage by 3.17%.
The diff coverage is 62.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   37.15%   40.32%   +3.17%     
==========================================
  Files           9        9              
  Lines         323      372      +49     
  Branches       45       54       +9     
==========================================
+ Hits          120      150      +30     
- Misses        203      217      +14     
- Partials        0        5       +5     

Impacted Files	Coverage Δ
zyte_api/utils.py	68.75% <61.22%> (-24.59%)	⬇️
zyte_api/aio/client.py	27.39% <100.00%> (ø)

[kmike]

hey @Gallaecio! Do you know if there is any way to prevent double-encoding in this function?

In [1]: from w3lib.url import safe_download_url, safe_url_string, canonicalize_url

In [2]: safe_url_string("https://www.example.com/product/hometales-pack-of-2-sky/626857878294#bcrumbSearch:pillow%20cover")
Out[2]: 'https://www.example.com/product/hometales-pack-of-2-sky/626857878294#bcrumbSearch:pillow%20cover'

vs

In [1]: from zyte_api.utils import _safe_url_string

In [2]: _safe_url_string("https://www.example.com/product/hometales-pack-of-2-sky/626857878294#bcrumbSearch:pillow%20cover")
Out[2]: 'https://www.example.com/product/hometales-pack-of-2-sky/626857878294#bcrumbSearch:pillow%2520cover'

The issue is that sometimes input URLs are encoded, and sometimes they are not. w3lib's safe_url_string tries support both. It seems that after this change we might break the client library for users who send the percent-escaped URLs (e.g. the website itself may already provide encoded versions).

[Gallaecio]

Great catch, let me fix that and add a test for it.

[Gallaecio]

Rather than fixing my implementation here, I am thinking I should clean up and extend the corresponding tests on w3lib, then update w3lib to support the URL limitations of as many servers as possible (including Zyte API’s), and then use upstream safe_url_string here instead of a fork.

[kmike]

Thanks @Gallaecio!