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

use only hostname as route target for export service #3

Merged
merged 1 commit into from Sep 7, 2017

Conversation

Projects
None yet
2 participants
@sevenEng
Contributor

sevenEng commented Sep 7, 2017

should strip url scheme and port number
when access token is requested from arbiter, it may wrongly respond 401 due to this

@Toshbrown

This comment has been minimized.

Show comment
Hide comment
@Toshbrown

Toshbrown Sep 7, 2017

Contributor

Under what conditions may it wrongly respond 401? Will this affect any other components?

As far as I know, thay all used the full URL with protocol and port as their target.

Contributor

Toshbrown commented Sep 7, 2017

Under what conditions may it wrongly respond 401? Will this affect any other components?

As far as I know, thay all used the full URL with protocol and port as their target.

@sevenEng

This comment has been minimized.

Show comment
Hide comment
@sevenEng

sevenEng Sep 7, 2017

Contributor

I'm testing using twitter-sentiment app. When it asks for access token for export service, it ends up using host name as route target: https://github.com/me-box/lib-node-databox/blob/master/lib/utils.js#L70

Not sure what's the intended format, but all the other configs for app/driver seem using just hostname, for example:
https://github.com/me-box/core-container-manager/blob/master/src/container-manager.js#L420
https://github.com/me-box/core-container-manager/blob/master/src/container-manager.js#L426
among others. So other components are not affected.

Contributor

sevenEng commented Sep 7, 2017

I'm testing using twitter-sentiment app. When it asks for access token for export service, it ends up using host name as route target: https://github.com/me-box/lib-node-databox/blob/master/lib/utils.js#L70

Not sure what's the intended format, but all the other configs for app/driver seem using just hostname, for example:
https://github.com/me-box/core-container-manager/blob/master/src/container-manager.js#L420
https://github.com/me-box/core-container-manager/blob/master/src/container-manager.js#L426
among others. So other components are not affected.

@sevenEng sevenEng changed the title from use only hostname as route target to use only hostname as route target for export service Sep 7, 2017

@Toshbrown

This comment has been minimized.

Show comment
Hide comment
@Toshbrown

Toshbrown Sep 7, 2017

Contributor

Your right! good spot. Merging!

Contributor

Toshbrown commented Sep 7, 2017

Your right! good spot. Merging!

@Toshbrown Toshbrown merged commit e914267 into me-box:master Sep 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment