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

Accept both levels of invite escaping #2838

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Accept both levels of invite escaping #2838

wants to merge 2 commits into from

Conversation

bemasc
Copy link
Contributor

@bemasc bemasc commented Dec 5, 2016

Currently only double-escaped invites are supported.


This change is Reviewable

Currently only double-escaped invites are supported.
@trevj
Copy link
Contributor

trevj commented Dec 6, 2016

Funny, you discovered the same bug I spent a few hours tracing towards the end of last week.

In cases like this, you can't over-comment - definitely explain that us running jsurl on a JSON-encoded string was just a totally dumb bug that we're kinda stuck supporting now (argh).

@bemasc
Copy link
Contributor Author

bemasc commented Dec 6, 2016

OK, commented.

Copy link
Contributor

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move the url parsing logic to a separate function? Maybe add a test?

@fortuna fortuna removed their assignment May 4, 2017
@fortuna
Copy link
Contributor

fortuna commented May 4, 2017

I guess we can close this PR

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

Successfully merging this pull request may close these issues.

3 participants