Skip to content

Implement a safe_url based on all standards#221

Draft
Gallaecio wants to merge 36 commits into
scrapy:masterfrom
Gallaecio:safer-url
Draft

Implement a safe_url based on all standards#221
Gallaecio wants to merge 36 commits into
scrapy:masterfrom
Gallaecio:safer-url

Conversation

@Gallaecio

@Gallaecio Gallaecio commented Feb 13, 2024

Copy link
Copy Markdown
Member

Continuation of #201.

Fixes #193.

Tasks:

  • Provide an alternative implementation for safe_url_string based on the URL living standard
  • Add missing typing information
  • Provide comprehensive test coverage for both implementations, highlighting the differences, and in the process make the new implementation support also RFC 2396 (used by java.net.URI) and RFC 3986 (the previous standard, still popular).
  • Ensure the following issues are addressed by the new implementation:
  • Have the URL living standard parse implementation mostly pass the upstream tests.
  • Improve performance
    The new implementation is currently around 3-4 times slower than the previous implementation for test cases where both have the same, non-error outcome.
  • Address to-dos in test_parse_url.
  • Clean up the implementation: cleaner code, docstrings…
  • Discuss what to do API-wise (deprecate safe_url_string in favor of safe_url? Remove safe_url and make safe_url_string use the new implementation?

@Gallaecio

Copy link
Copy Markdown
Member Author

I am having a hard time lowering the gap below the current one, which is 4-3 times slower than the previous implementation 😞

@Gallaecio

Copy link
Copy Markdown
Member Author

Looking at scrapy/scrapy#1306, maybe we could try going for https://chromium.googlesource.com/chromium/src/+/HEAD/url/ .

@Gallaecio

Copy link
Copy Markdown
Member Author

@kmike

kmike commented Mar 16, 2024

Copy link
Copy Markdown
Member

An interesting project: https://github.com/TkTech/can_ada

@albertedwardson

Copy link
Copy Markdown
Contributor

can be useful: https://github.com/crate-py/url

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

safe_url_string handling IPv6 URLs

3 participants