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

Request Bodies with "o" Object Should be Encoded #93

Closed
danielcliu-ifixit opened this issue Aug 8, 2023 · 6 comments
Closed

Request Bodies with "o" Object Should be Encoded #93

danielcliu-ifixit opened this issue Aug 8, 2023 · 6 comments

Comments

@danielcliu-ifixit
Copy link

danielcliu-ifixit commented Aug 8, 2023

Overview

I'm assuming this is the correct repo to write an issue found within /_vercel/insights/script.js. If not please let me know or point me in the right direction.

in script.js we make a fetch call to /_vercel/insights/ endpoints like so

await fetch(`${n}/${"pageview" === i ? "view" : "event"}`, {
           method: "POST",
           keepalive: !0,
           headers: {
               "Content-Type": "application/json"
           },
           body: JSON.stringify(f)
       })

however the body f is defined by

f = {
       o: d,
       sv: "0.1.2",
       sdkn: null != (r = null == t ? void 0 : t.getAttribute("data-sdkn")) ? r : void 0,
       sdkv: null != (l = null == t ? void 0 : t.getAttribute("data-sdkv")) ? l : void 0,
       ts: Date.now(),
       ...null != a && a.withReferrer && !s ? {
           r: u
       } : {},
       ..."event" === i && o && {
           en: o.name,
           ed: o.data
       }
   };

and d = location.href. The reason this is an issue is the /_vercel/insights/ endpoints seem to anticipate that o is already encoded, and if its not returns an error that looks like
{"statusCode":400,"error":"Bad Request","message":"body/href must match format \"uri\""}.

If you want to see evidence of this error you can search "vercel" requests in dev tools -> network when visiting a page like

https://www.ifixit.com/products/galaxy-note10-plus-replacement-battery?exampleVercelError={hello}

whereas
https://www.ifixit.com/products/galaxy-note10-plus-replacement-battery?exampleVercelError=hello has those vercel calls work fine.

Proposed Solution

I think that body: JSON.stringify(encodeURI(f)) should solve this issue, or maybe getting the uri and then running encodeURIComponent on the params etc. Or if you have access to the vitals API endpoints code you can make it encode expected URIs on the server side, because I notice the same error occurring for the body key-value href for calls to
https://vitals.vercel-insights.com/v1/vitals

@danielbeardsley
Copy link

I think that body: JSON.stringify(encodeURI(f)) should solve this issue,

f is an object here. Perhaps you meant f.o = encodeURI(f.o);?

@danielcliu-ifixit
Copy link
Author

actually i think

f = {
       o: encodeURI(d),
       sv: "0.1.2",
       sdkn: null != (r = null == t ? void 0 : t.getAttribute("data-sdkn")) ? r : void 0,
       sdkv: null != (l = null == t ? void 0 : t.getAttribute("data-sdkv")) ? l : void 0,
       ts: Date.now(),
       ...null != a && a.withReferrer && !s ? {
           r: u
       } : {},
       ..."event" === i && o && {
           en: o.name,
           ed: o.data
       }
   };

would be best

@tobiaslins
Copy link
Collaborator

tobiaslins commented Aug 10, 2023

Hey @danielcliu-ifixit!

I don't think that we should use encodeURI in our script because if params are already encoded (as they should be in the URL) we would double encode them. For example when I search on ifixit.com for { the location is https://www.ifixit.com/Search?query=%7B and when we would use encodeURI it would be https://www.ifixit.com/Search?query=%257B so the actual value would not be recoverable easily.

My question is: why do you have the unencoded character in the URL in the first place?

One possible solution would be to use beforeSend in the script to encode the params before sending them.

<Analytics beforeSend={(event) => {
   if(event.url.includes("exampleVercelError") {
     return { 
        ...event,
        url: encodeURI(event.url)
     }
   }
   return event;
 }
} />

Let me know what you think!

@sterlinghirsh
Copy link

The curly brace is being added by another analytics package (matomo). Maybe it could be encoded, but as far as I can tell, urls with curly braces in the query strings are still valid. The page doesn't seem to have a problem loading, so it seems odd that vercel analytics would decide that our url is invalid.

I agree double-encoding the url isn't the right answer, but the real problem is that the analytics api gives a 400 on a valid url because the uri validator is not conforming to the url spec. https://url.spec.whatwg.org/#query-percent-encode-set A fix to that validator should resolve this issue.

@tobiaslins
Copy link
Collaborator

Thanks for bringing that up.
We just deployed a fix for that and you should now not see any 400's anymore. Let me know if you can confirm @danielcliu-ifixit

@danielcliu-ifixit
Copy link
Author

@tobiaslins thanks for the fix! My issue seems to be resolved.

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

No branches or pull requests

4 participants