fix: Don't track hash params by default

[benjackwhite]

Changes

Hash params are typically not necessary to be tracked and often even undeseriable as they can be used for storing tokens etc. that never leave the browser.

I don't think we should track these by default. This is technically a breaking change so a little hard to guage but I strongly feel like the default should be not to track

We probably want to offer a config option to allow this...

Checklist

• Tests for new code (see advice on the tests we use)
• Accounted for the impact of any changes across different browsers
• Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
posthog-js	✅ Ready (Inspect)	Visit Preview	Aug 29, 2024 7:50am

[github-actions]

Size Change: +770 B (+0.07%)

Total Size: 1.17 MB

Filename	Size	Change
dist/array.full.js	335 kB	+188 B (+0.06%)
dist/array.js	155 kB	+194 B (+0.13%)
dist/main.js	156 kB	+194 B (+0.12%)
dist/module.js	155 kB	+194 B (+0.13%)

ℹ️ View Unchanged

Filename	Size
dist/exception-autocapture.js	10.4 kB
dist/recorder-v2.js	110 kB
dist/recorder.js	110 kB
dist/surveys-preview.js	59.8 kB
dist/surveys.js	66 kB
dist/tracing-headers.js	8.26 kB
dist/web-vitals.js	5.79 kB

_{compressed-size-action}

[marandaneto]

I recall people complaining (in the past) about query params as well, so masking query param values would be also good, eg:

function maskUrl(url: string): string {
    try {
        const parsedUrl = new URL(url);

        parsedUrl.searchParams.forEach((_, key) => {
            parsedUrl.searchParams.set(key, 'masked');
        });

        if (parsedUrl.hash) {
            parsedUrl.hash = 'masked';
        }

        return parsedUrl.toString();
    } catch (e) {
        console.error("Invalid URL provided:", e);
        return url;
    }
}

const originalUrl = "https://www.google.com?query=123&login=true#456";
const maskedUrl = maskUrl(originalUrl);
console.log(maskedUrl); // "https://www.google.com/?query=masked&login=masked#masked"

Not a blocker for this PR, just remembering that I had to do something like this in the past.

[pauldambra]

I know it's not this PR but "info" makes me sad 🤣
we should rename it "misc" :p

[pauldambra]

almost feels like we should capture the hash params separately so that if folk really want them they can capture them

instead of having href and hash

I don't think that massively affects queries so no strong opinion but would sit nicely to have query_params, pathname, and hash_params as separate things

(no need to take that as blocking here i think generally we could do with better url handling)

[pauldambra]

agree it would be nice for this to be configurable...
arguably if we allow the config we should add a scrub function so folk can capture via allowlist or denylist so they don't capture sensitive info unless they really want to

since it looks like hash params are not being queried for by many folk we can probably not take on the complexity now and ask them if they need it and add it on demand

[benjackwhite]

agree it would be nice for this to be configurable... arguably if we allow the config we should add a scrub function so folk can capture via allowlist or denylist so they don't capture sensitive info unless they really want to

since it looks like hash params are not being queried for by many folk we can probably not take on the complexity now and ask them if they need it and add it on demand

I feel like this isn't specific to URLs. We should just have a hook for people to hook into and modify the final captured event payload. That way they can do whatever level of modifications they want and avoids the need for more and more complex config options tbh...

[pauldambra]

We should just have a hook for people to hook into and modify the final captured event payload. That way they can do whatever level of modifications they want and avoids the need for more and more complex config options tbh...

+100 🧠

[robbie-c]

We should just have a hook for people to hook into and modify the final captured event payload. That way they can do whatever level of modifications they want and avoids the need for more and more complex config options tbh...

I'm in favour of this too. This would be cool:

import {stripQueryParams, stripProperties} from "@posthog/js-transforms"

posthog.init("my-token", {
  transforms: [
    stripQueryParams(["some-oauth-token", "some-ad-id"]),
    stripProperties(["$browser"])
  ]
}

[posthog-bot]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

[posthog-bot]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.