tradingstrategy-ai / frontend

Web frontend for TradingStrategy.ai
https://tradingstrategy.ai
128 stars 23 forks source link

Invalid time bucket in URL generates UnhandledRejection error #790

Closed kenkunz closed 2 months ago

kenkunz commented 2 months ago

Issue

This issue was identified via Sentry error reporting: https://sentry.tradingstrategy.ai/organizations/tradingstrategy/issues/596

The issue occurs on the pair details page when a # value is included on the URL that isn't a valid time bucket (e.g., 1h, 4h, 1d).

Steps to reproduce

  1. Open a valid trading pairs page – e.g., WISE-ETH
  2. Append a # followed by an invalid value to the URL – e.g., #foo
  3. Note unexpected behavior / errors:
    • chart spinners spin indefinitely
    • error console shows 422 response to GET request, as well as Uncaught (in promise) error

Why this matters

In general, we shouldn't care much if someone constructs a bogus URL and they get an error as a result (as long as it isn't causing other harmful side effects).

In this instance, we should care because the invalid URL's seem to be commonly recurring "in the wild". Here's the stats from Sentry:

image

Looking through various occurrences of this error, the content after the # follows a common pattern. Here's an example fragment:

:~:text=The%20price%20of%20SIDRA%20in,Dollar%20for%20the%20last%2024h.

Decoded:

:~:text=The price of SIDRA in,Dollar for the last 24h.

… which matches some text fragments found on the pair details page.

Why is this happening??

This seemed really strange. Something must be causing the common "bad URL" pattern to occur. What??

❌ Hypothesis 1: vulnerability scanning bots

I ruled this out pretty quickly b/c the content above doesn't include the character strings one would expect for some type of JS or SQL injection attack vector.

❌ Hypothesis 2: malformed link on our page

Since the URL fragments match copy fragments shown on the pair details page, I considered the possibility that we had a malformed link on our page somewhere, such that when you click on it, it appends the bogus hash fragment to the URL. I ruled this out quickly by checking the code and testing all the proximate links on the page.

❌ Hypothesis 3: search engines appending the fragment

Perhaps search engines are appending the search query content to the URL? This seemed unlikely, but easy to rule out. I tried entering some example fragments in various search engines and then testing the result links. No dice!

The search results pointed me in the right direction, though…

✅ Hypothesis 4: Text fragment directives

Aha!

TIL: Chrome and other browsers support a feature called Text fragments whereby a user can highlight some text on a page and then copy a link to the highlighted text. The browser appends the text fragment directive to the URL hash. When clicking on such a link, supporting browsers will recognize the text fragment directive and highlight (and scroll to) the matching text.

Screenshot 2024-07-30 at 4 44 45 PM

Apparently, Trading Strategy users are using this feature to link back to the price information section on trading pairs 🎉.

The bug / error actually occurs when someone clicks one of these links with an unsupported browser (one that doesn't support text fragment directives). Supported browsers strip the directive after highlighting the text, so the value doesn't show up when accessing the URL hash in JavaScript. But unsupported browsers (e.g, Firefox) don't know what to do with the directive, so it behaves like any other URL hash value.

Somehow I've managed to remain oblivious to this feature for the past 4 years 😵‍💫🤦.

Resolution

The fix for this is to use an explicit URL param (?timeBucket=4h) instead of the URL hash fragment for persisting the chart time bucket value. In addition, the value of this param is validated against a list of valid time buckets and ignored if it's not valid (falling back to default 1d).