- Notifications
You must be signed in to change notification settings - Fork215
🐛 FIX: Double encoded ampersands in query params#929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
🐛 FIX: Double encoded ampersands in query params#929
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecov-commenter commentedMay 4, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## master #929 +/- ##==========================================+ Coverage 90.13% 90.15% +0.02%========================================== Files 24 24 Lines 3416 3423 +7 ==========================================+ Hits 3079 3086 +7 Misses 337 337
Flags with carried forward coverage won't be shown.Click here to find out more. ☔ View full report in Codecov by Sentry. |
| HTMLTranslator.special_chars_no_amp= {# needed by encode_fixed | ||
| k:vfork,vinHTMLTranslator.special_characters.items()ifk!=ord("&") | ||
| } | ||
| HTMLTranslator.encode=encode_fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm afraid this is not something that cannot be merged; it would break restructuredtext parsing.
Monkey patching is done strictly as a last resort, and only for where the patch does not affect upstream use
Thiscloses#760.
MyST already escapes the
&in the query params, but docutils escapes them again when rendering HTML, leading to broken query params.becomes
instead of
These changes to the docutils parser override the
HTMLTranslator.encodefunction so already encoded ampersands are not encoded again.Since the issue lies in the interaction with docutils, I think it makes sense to fix it here.
Also, simply removing the
escapeHtmlcall in MyST makes the sphinx app doctree XML files invalid because the ampersands aren't escaped.