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

Endpoint with trailing slash #175

Open
wes-novack opened this issue Jan 5, 2021 · 5 comments · Fixed by vaultaire/vaultaire#23
Open

Endpoint with trailing slash #175

wes-novack opened this issue Jan 5, 2021 · 5 comments · Fixed by vaultaire/vaultaire#23

Comments

@wes-novack
Copy link
Contributor

Hello! For the VAULT_ADDR endpoint value, if the caller provides a value that includes a trailing slash, eg; http://127.0.0.1:8200/, the library currently does not handle it well.

We can see that the full uri is constructed here: https://github.com/kr1sp1n/node-vault/blob/70097269d35a58bb560b5290190093def96c87b1/src/index.js#L91

...with a trailing slash concatenated into the uri after the client.endpoint.

We would like to submit a PR that addresses the problem with a client.endpoint value that includes a trailing slash, by automatically handling it by removing that trailing slash from the endpoint value.

Thoughts/comments/discussion appreciated.

wes-novack added a commit to pluralsight/node-vault that referenced this issue Jan 5, 2021
Co-authored-by: Jason Nguyen <jason-nguyen@pluralsight.com>
Co-authored-by: Bret Hubbard <bret-hubbard@pluralsight.com
wes-novack added a commit to pluralsight/node-vault that referenced this issue Jan 5, 2021
Co-authored-by: Jason Nguyen <jason-nguyen@pluralsight.com>
Co-authored-by: Bret Hubbard <bret-hubbard@pluralsight.com
@GoFightNguyen
Copy link
Contributor

@kr1sp1n any thoughts on this?

@Aenima4six2
Copy link

I had to normalize all my URLs before I handed them off. Would be great if the package did that for us.

MichaelSp added a commit to vaultaire/vaultaire that referenced this issue Apr 24, 2022
fixes nodevault#175

Co-authored-by: Jason Nguyen <jason-nguyen@pluralsight.com>
Co-authored-by: Bret Hubbard <bret-hubbard@pluralsight.com
MichaelSp added a commit to vaultaire/vaultaire that referenced this issue Apr 24, 2022
based on nodevault#176

fixes nodevault#175

Co-authored-by: Jason Nguyen <jason-nguyen@pluralsight.com>
Co-authored-by: Bret Hubbard <bret-hubbard@pluralsight.com
@aviadhahami
Copy link
Collaborator

Hey @wes-novack! sry for the (very) late response :X

Do you mind creating this PR so I can merge it?

@GoFightNguyen
Copy link
Contributor

@aviadhahami This issue is addressed in #176

@aviadhahami
Copy link
Collaborator

@GoFightNguyen thx! just saw the PR was updated but codecov & travis are stuk

I'll await @kr1sp1n to give me perms to force rebuild and merge 👍
thx for the help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants