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

Fix request parsing #214

Closed
wants to merge 2 commits into from
Closed

Fix request parsing #214

wants to merge 2 commits into from

Conversation

Galiley
Copy link

@Galiley Galiley commented Aug 11, 2021

Split already parsed query parameters by the Application (Express for instance)

@anttiviljami
Copy link
Member

Hi @Galiley, could you explain the rationale for the first change?

Maybe add a test to explain the case you’re fixing with this change

@Galiley
Copy link
Author

Galiley commented Aug 21, 2021

The reason behind that is kinda related to #144

@anttiviljami
Copy link
Member

Any chance you could device a test that demonstrates this to document better what's being fixed?

Since this change affects how all requests are parsed and while I don't see any huge issues here, I'm a little uncomfortable merging without fully understanding the need for this change.

@Galiley
Copy link
Author

Galiley commented Aug 27, 2021

While trying to make some tests, I've found out that the change I've made in the handleRequest method was kinda useless.
That's why I'm reverting my changes

@anttiviljami
Copy link
Member

Sorry, I'm really struggling to understand what problem this PR is solving so I've decided to close it. Feel free to reopen and give a better explanation of why this is needed.

@Galiley Galiley deleted the request-parsing-fix branch January 6, 2022 16:46
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.

2 participants