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

MasterApi: HTTP header names are case insensitive #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pini-gh
Copy link

@pini-gh pini-gh commented Mar 2, 2021

Fixes #7.

@nviennot
Copy link
Member

That should not be needed.
https://github.com/tmate-io/tmate-websocket/blob/master/lib/tmate/master_api.ex#L7 enables the json transforms
https://github.com/tmate-io/tmate-websocket/blob/master/lib/tmate/util/json_api.ex#L37 decoding should happen here.

If your PR is fixing a bug, how can I reproduce it?

@pini-gh
Copy link
Author

pini-gh commented Mar 21, 2021

If your PR is fixing a bug, how can I reproduce it?

This is explained in #7. See the console part.

@nviennot
Copy link
Member

image

Seems fine here

@pini-gh
Copy link
Author

pini-gh commented Mar 21, 2021

Thanks to your pointers I've found out where the actual problem lies:
https://github.com/tmate-io/tmate-websocket/blob/master/lib/tmate/util/json_api.ex#L35

For the tmate-websocket instance I used the same value for MASTER_BASE_URL and USER_FACING_BASE_URL. Then the /named_session_prefix request was going through my reverse-proxy, which uses Content-Type instead of content-type as header name.

It works if I set MASTER_BASE_URL to the internal URL http://tmate-master:4000/. In this case the header name is content-type (all lowercase). But it might be worth making the header name lookup case insensitive:

        content_type_hdr = Enum.find(headers, fn {name, _} -> String.downcase(name) == "content-type" end)

@pini-gh pini-gh changed the title MasterApi: JSON decode before using body data MasterApi: HTTP header names are case insensitive Mar 22, 2021
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.

Tmate.MasterApi.get_named_session_prefix() fails parsing json result
2 participants