Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't decode querystring while adding apiExpanders #4719

Merged
merged 5 commits into from
Apr 20, 2023

Conversation

davisagli
Copy link
Sponsor Member

addExpandersToPath parses the current querystring, makes modifications, and then stringifies it again. stringify is called with encode: false so we need to call parseUrl with decode: false to match.

This should be backwards compatible unless there are expanders whose name includes an urlencoded character, or code somewhere which is already doing extra encoding to work around this bug.

Fixes #4718

@netlify
Copy link

netlify bot commented Apr 19, 2023

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 60e10d3
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/6440ef631c9a920008f5b42f

@cypress
Copy link

cypress bot commented Apr 19, 2023

Passing run #4996 ↗︎

0 493 20 0 Flakiness 0

Details:

More tests
Project: Volto Commit: 60e10d31b6
Status: Passed Duration: 11:00 💡
Started: Apr 20, 2023 7:56 AM Ended: Apr 20, 2023 8:07 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@davisagli
Copy link
Sponsor Member Author

I don't understand the failing unit tests yet. They are failing only in CI on Node 18, they work for me locally.

@davisagli
Copy link
Sponsor Member Author

Looks like the test failures are a regression in Node 18.16.0: nodejs/node#47563

@sneridagh
Copy link
Member

@davisagli I added more tests just to be 100% sure.

@sneridagh sneridagh merged commit 4151254 into master Apr 20, 2023
@sneridagh sneridagh deleted the fix-api-querystring branch April 20, 2023 08:15
sneridagh added a commit that referenced this pull request Apr 20, 2023
Co-authored-by: Victor Fernandez de Alba <sneridagh@gmail.com>
ionlizarazu pushed a commit that referenced this pull request Apr 20, 2023
Co-authored-by: Victor Fernandez de Alba <sneridagh@gmail.com>
sneridagh added a commit that referenced this pull request Apr 20, 2023
Co-authored-by: David Glick <david@glicksoftware.com>
sneridagh added a commit that referenced this pull request May 10, 2023
* master: (83 commits)
  Apply suggestion from browser for password field (#4524)
  (fix):Object.normaliseMail: Cannot read properties of null (#4558)
  Fix link in Volto, remove from linkcheck ignore in Documentation v6.0 (#4742)
  added documentation regarding the static middleware #4518 (#4736)
  Closes issue #4567 (#4570)
  Fix whitespace in locales created by the generator (#4737)
  Tidy up from synch with 16.x.x (#4728)
  Release notes from 16.20.2, 16.20.3, 16.20.4 (#4729)
  Use new URL 6.docs.plone.org (#4726)
  Security upgrade for momentjs (#4715)
  Don't decode querystring while adding apiExpanders (#4719)
  (Synchronize redundant block id in listing block on pasting) Fix duplicating listing block (#4239)
  Translate add-on control panel (cleaned up PR) (#4620)
  Fix Move to top of folder ordering in folder content view by searchin… (#4709)
  Fix robot.txt - the sitemap link should respect x-forwarded headers (#4704)
  Refactor faulty reorder elements in ObjectBrowserList widget (#4703)
  Release changelog notes for 16.20.1
  Release 17.0.0-alpha.5
  Generate a split sitemap (also fix robots.txt) (#4639)
  Fix search block in edit mode re-queries multiple blocks with an empty search text (#4694)
  ...
sneridagh added a commit that referenced this pull request May 11, 2023
* master: (185 commits)
  fix: unresponsive add page (#4507)
  Apply suggestion from browser for password field (#4524)
  (fix):Object.normaliseMail: Cannot read properties of null (#4558)
  Fix link in Volto, remove from linkcheck ignore in Documentation v6.0 (#4742)
  added documentation regarding the static middleware #4518 (#4736)
  Closes issue #4567 (#4570)
  Fix whitespace in locales created by the generator (#4737)
  Tidy up from synch with 16.x.x (#4728)
  Release notes from 16.20.2, 16.20.3, 16.20.4 (#4729)
  Use new URL 6.docs.plone.org (#4726)
  Security upgrade for momentjs (#4715)
  Don't decode querystring while adding apiExpanders (#4719)
  (Synchronize redundant block id in listing block on pasting) Fix duplicating listing block (#4239)
  Translate add-on control panel (cleaned up PR) (#4620)
  Fix Move to top of folder ordering in folder content view by searchin… (#4709)
  Fix robot.txt - the sitemap link should respect x-forwarded headers (#4704)
  Refactor faulty reorder elements in ObjectBrowserList widget (#4703)
  Release changelog notes for 16.20.1
  Release 17.0.0-alpha.5
  Generate a split sitemap (also fix robots.txt) (#4639)
  ...
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.

API middleware mangles urlencoding in querystring
2 participants