Forked from #3085360: RouteProvider::getRouteCollectionForRequest() can poison query string of next request, where it was originally reported.

CommentFileSizeAuthor
#8 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-6.patch9.21 KBjosephdpurcell
#8 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-6-test-only.patch6.02 KBjosephdpurcell
#6 jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6-test-only.patch6.02 KBjosephdpurcell
#6 jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6.patch9.21 KBjosephdpurcell
#5 interdiff-4-5.txt562 bytesjosephdpurcell
#5 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-5.patch6.02 KBjosephdpurcell
#4 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-4.patch9.21 KBjosephdpurcell
#4 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-4-test-only.patch6.01 KBjosephdpurcell
#4 interdiff-3-4.txt593 bytesjosephdpurcell
#3 interdiff-2-3.txt801 bytesjosephdpurcell
#3 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-3.patch9.2 KBjosephdpurcell
#3 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-3-test-only.patch6.01 KBjosephdpurcell
#2 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-2-test-only.patch5.93 KBjosephdpurcell
#2 jsonapi-9.1.0-fix-poison-query-string-cache-3187763-2.patch9.12 KBjosephdpurcell

Comments

josephdpurcell created an issue. See original summary.

josephdpurcell’s picture

josephdpurcell commented
StatusFileSize
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-2.patch9.12 KB
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-2-test-only.patch5.93 KB
josephdpurcell’s picture

josephdpurcell commented
StatusFileSize
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-3-test-only.patch6.01 KB
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-3.patch9.2 KB
new interdiff-2-3.txt801 bytes

Retrying to fix the core_version_requirement error in the test.

josephdpurcell’s picture

josephdpurcell commented
StatusFileSize
new interdiff-3-4.txt593 bytes
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-4-test-only.patch6.01 KB
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-4.patch9.21 KB

Retrying to fix the lack of return type void error.

josephdpurcell’s picture

josephdpurcell commented
StatusFileSize
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-5.patch6.02 KB
new interdiff-4-5.txt562 bytes

Retrying to fix the modules must be declared protected. (Side note: why do these errors not show up when I run tests locally?)

josephdpurcell’s picture

josephdpurcell commented
Status: Active » Needs review
StatusFileSize
new jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6.patch9.21 KB
new jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6-test-only.patch6.02 KB

It worked! Ok, we can ignore all the previous tests. Here's the real deal:

jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6.patch -- this should pass the test
jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6-test-only.patch -- this should fail saying it expected application/json but got text/html

Setting to needs review.

Status: Needs review » Needs work

The last submitted patch, 6: jsonapi-9.1.0-fix-poison-query-string-cache-3085360-6-test-only.patch, failed testing. View results

josephdpurcell’s picture

josephdpurcell commented
StatusFileSize
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-6-test-only.patch6.02 KB
new jsonapi-9.1.0-fix-poison-query-string-cache-3187763-6.patch9.21 KB

Meh. I had the wrong ticket ID in the patch name. Uploading correctly named patches now.

josephdpurcell’s picture

josephdpurcell commented
Status: Needs work » Needs review

Great, that worked as expected! Setting to Needs Review.

bradjones1’s picture

bradjones1
Primary language English
commented
Title: JSON API ONLY: RouteProvider::getRouteCollectionForRequest() can poison query string of next request » Use api_json format on exception responses for api_json resources
Issue summary: View changes
Status: Needs review » Closed (duplicate)

Updating the title and description to match what I think your intent is here; as mentioned in our discussions over at #3085360: RouteProvider::getRouteCollectionForRequest() can poison query string of next request I think this is a separate issue, judging by the test introduced in your patch. I've tried to encapsulate what I think your goal is, in the new title and description.

Boils down to: "Use api_json format on exception responses for api_json resources" which I think is a duplicate of #3127428: HTTP 401 responses return HTML, not JSON:API error objects.