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

Avoid double slash in url when client hostname has tailing slash #738

Merged
merged 7 commits into from
Jun 1, 2020

Conversation

hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Feb 25, 2020

When GRPC client hostname has tailing '/', the final request url contains '//' which cannot be processed properly by some servers, e.g. grpc-dotnet-web

@stanley-cheung
Copy link
Collaborator

I am so sorry. I keep looking at this and thought, how would this compile? I will have to look up what this URL class is and how it will compile, in C++. Only did I suddenly realize - it's generated code. This is a javascript class. facepalm.

Thanks for the contribution. But tests failed. I believe there's a missing comma after toString().

Sorry about the delay.

@hanabi1224
Copy link
Contributor Author

Good catch! Fixed, please take another look

Copy link
Collaborator

@stanley-cheung stanley-cheung left a comment

Choose a reason for hiding this comment

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

LGTM, and thanks for the contribution!

@stanley-cheung stanley-cheung merged commit 825dd2a into grpc:master Jun 1, 2020
@hanabi1224 hanabi1224 deleted the avoid_double_slash branch June 2, 2020 09:00
@Fl0pZz
Copy link

Fl0pZz commented Jun 23, 2020

@stanley-cheung @hanabi1224 this PR drop support uri like: https://example.com/api/grpc/v1/...

See examples:
https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

@stanley-cheung
Copy link
Collaborator

@Fl0pZz sorry I am a bit unclear on this - do you mean that you are passing the hostname parameter with the 3 dots at the end?

@Fl0pZz
Copy link

Fl0pZz commented Jun 23, 2020

@Fl0pZz sorry I am a bit unclear on this - do you mean that you are passing the hostname parameter with the 3 dots at the end?

Nope :)
I mean host name like this:
Hostname: https://example.com/api/grpc/v1

And in browser dev tools i see request: https://example.com/api/grpc/v1/MyProject.listEntities

@stanley-cheung
Copy link
Collaborator

@Fl0pZz and what's wrong with the above?

@Fl0pZz
Copy link

Fl0pZz commented Jun 23, 2020

@Fl0pZz and what's wrong with the above?

From mdn: https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

const hostname = 'https://example.com/api/grpc/v1';
const uri = new URL('/MyProject.listEntities', hostname);

console.log(uri.toString()) // -> https://example.com/MyProject.listEntities

however i expected https://example.com/api/grpc/v1/MyProject.listEntities

stanley-cheung added a commit to stanley-cheung/grpc-web that referenced this pull request Jun 23, 2020
@stanley-cheung stanley-cheung mentioned this pull request Jun 23, 2020
stanley-cheung added a commit that referenced this pull request Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants