vercel / analytics

Privacy-friendly, real-time traffic insights
https://vercel.com/analytics
Mozilla Public License 2.0
422 stars 26 forks source link

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

Closed danielcliu-ifixit closed 1 year ago

danielcliu-ifixit commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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