Skip to content
This repository was archived by the owner on Jul 12, 2022. It is now read-only.

Prevent JavaScript error on malformed URI#11

Merged
KittyGiraudel merged 1 commit intomasterfrom
feature/decode-uri-component
Jan 18, 2018
Merged

Prevent JavaScript error on malformed URI#11
KittyGiraudel merged 1 commit intomasterfrom
feature/decode-uri-component

Conversation

@KittyGiraudel
Copy link
Copy Markdown
Contributor

No description provided.

Tom-Bonnike
Tom-Bonnike previously approved these changes Jan 18, 2018
Copy link
Copy Markdown
Contributor

@Tom-Bonnike Tom-Bonnike left a comment

Choose a reason for hiding this comment

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

Yea. Guess we don’t need a test for it?

@KittyGiraudel KittyGiraudel force-pushed the feature/decode-uri-component branch 2 times, most recently from d228369 to 29e3215 Compare January 18, 2018 09:10
@KittyGiraudel KittyGiraudel force-pushed the feature/decode-uri-component branch from 29e3215 to 09f30dd Compare January 18, 2018 09:12
@KittyGiraudel
Copy link
Copy Markdown
Contributor Author

@Tom-Bonnike We do. Added.

})

it('should not throw if failing to decode query from request', function () {
var request = getMockedRequest({ query: { locale: '/at//%c0%ae%c0%ae%c0%ae%c0%c0%a%c0%c0%c0%c0%af%c0%af%c0%af%c0%c0%ae%c0%c0%ae%c0%ae%c0%afwindowswin.ini' } })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is an example of URL used to DDoS us.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know. :)

return request.query[key] ? decodeURIComponent(request.query[key]) : null
} catch (error) {
return null
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I prefered when you let it fail before but doesn’t matter 😄

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well but that’s the entire problem.

Copy link
Copy Markdown
Contributor

@Tom-Bonnike Tom-Bonnike Jan 18, 2018

Choose a reason for hiding this comment

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

No. What I meant is, before you rebased you wrote:

try {
  return decodeURIComponent(request.query[key])
} catch (error) {
  return null
}

Copy link
Copy Markdown
Contributor

@Tom-Bonnike Tom-Bonnike Jan 18, 2018

Choose a reason for hiding this comment

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

Oh but passing undefined doesn’t throw an error. Ok :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That doesn’t do the same thing, all tests were failing. decodeURIComponent(undefined) === 'undefined'.

@KittyGiraudel KittyGiraudel merged commit e0f8fb0 into master Jan 18, 2018
@KittyGiraudel KittyGiraudel deleted the feature/decode-uri-component branch January 18, 2018 09:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants