-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
fix: Patch paho-mqtt to apply fix for bug when cookies are disabled #10841
Conversation
var match = uri.match( | ||
/^(wss?):\/\/((\[(.+)\])|([^\/]+?))(:(\d+))?(\/.*)$/ | ||
); |
Check failure
Code scanning / CodeQL
Polynomial regular expression used on uncontrolled data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preferring to keep this consistent with the vendor library implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Thanks @stocaaro 🎖️
Codecov Report
@@ Coverage Diff @@
## main #10841 +/- ##
==========================================
- Coverage 85.88% 81.30% -4.58%
==========================================
Files 197 198 +1
Lines 18373 19510 +1137
Branches 3909 4208 +299
==========================================
+ Hits 15779 15862 +83
- Misses 2520 3358 +838
- Partials 74 290 +216
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
db0657a
to
e8d34c5
Compare
Original PR: eclipse-paho/paho.mqtt.javascript#247 Co-authored-by: Sam Dawson <[email protected]>
e8d34c5
to
8f74aea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Description of changes
paho-mqtt
raises an error breakingamplify-js
customer experiences when customers have cookies disabled (as detailed in this issue).This change adopts the proposed fix as a patch directly in the PubSub category code to fix the experience.
Issue #, if available
#9185
Description of how you validated changes
The error is reproducible by:
I have verified that without this change, the result is the error:
After the change, my application works as normal allowing subscriptions to be created and published to, with the caveat that it will prompt for login on every reload (no cookies).
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.