8000 Compatibility for broken servers and additional authentication option instead of just basic by thorstenfreitag · Pull Request #77 · Links2004/arduinoWebSockets · GitHub
[go: up one dir, main page]
Skip to content

Compatibility for broken servers and additional authentication option instead of just basic #77

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 10, 2016
Merged

Compatibility for broken servers and additional authentication option instead of just basic #77

merged 2 commits into from
May 10, 2016

Conversation

thorstenfreitag
Copy link

Hi Markus,
Firstly thanks for this great library. I have made two changes and was hoping you could merge them into your library:

  1. I adopted the change you had already made for the "Upgrade" header to the "Connection" header. I looked at the RFC and Upgrade with upper case should be the expected value, but there seems to be a lot of "broken" servers out there (for instance everything based on TomCat) that send a lower case upgrade. the equalsIgnoreCase should work with both and make your library work with all servers.
  2. I changed the behavior of the setAuthorization(const char * auth) function. In the original you just took the passed value and appended it unmodified to the Authorization header, but it still had the BASIC in front of it. Therefore it would still only allow basic authentication. With the little change I made you could for instance pass "Bearer xxxxAuthTokenxxxx" to enable token based authentication on a lot of IoT platforms. If people really want to pass a pre-made Basic auth header then they can just use "BASIC xxxxxxxBasicAuthValuexxxxx" to achieve what the library currently does.

Thank you for considering the changes and thanks again.

Cheers,
Thorsten

PS. :Viele Grüße, habe gerade gesehen das "Germany" deine Location ist... ;-)

Thorsten Freitag added 2 commits May 9, 2016 00:11
Changed Header value to lower case upgrade, seems to fix connection i…
d36f7bb 
…ssues with SAP HCP IoT services. Changed setAuthorization(const char * auth) to send Auth header as is, without BASIC to enable oAuth tokens in header
Used case insensitive recognition for upgrade header. Should work as …
dd14850 
…before, but also with servers that wrongly use lower case upgrade in the header
@Links2004 Links2004 merged commit 93b0b9e into Links2004:master May 10, 2016
@Links2004
Copy link
Owner

Hello Thorsten,

I see no problem with the changes --> merged.
the setAuthorization change can give some problems for upgrading users so I will note it in the Release notes.

Grüße aus DE
Markus

@thorstenfreitag
Copy link
Author

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thorstenfreitag @Links2004
0