-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
Default headers example AUTH_TOKEN comment #3539
Merged
jasonsaayman
merged 7 commits into
axios:master
from
aliclark:default-headers-example-without-confidential-text
Jan 12, 2021
Merged
Default headers example AUTH_TOKEN comment #3539
jasonsaayman
merged 7 commits into
axios:master
from
aliclark:default-headers-example-without-confidential-text
Jan 12, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The existing example usage it isn't safe in the general case as it can lead to auth tokens being leaked to 3rd party endpoints by unexpectedly. This change instead gives an example using "axios.defaults.headers.common" to set the User-Agent, which is an equally helpful use-case to document. The 'Custom instance defaults' example just below the 'Global axios defaults' example shows a method to set the 'Authorization' header specific to a given API. I've also updated the variable in the 'Custom instance defaults' code to use a semantically more relevant name within that example.
jasonsaayman
approved these changes
Jan 11, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I know I am being a bit uptight but if you can change that wording then it will be perfect 💯
…ithub.com:aliclark/axios into default-headers-example-without-confidential-text
chinesedfan
suggested changes
Jan 12, 2021
See axios#1231 Credit @chinesedfan for pointing this out
chinesedfan
approved these changes
Jan 12, 2021
Thanks both for the feedback! I'm not able to merge as a collaborator so hope either of you can merge whenever is a good time. Thanks again and apologies for the drama. |
This was referenced Sep 9, 2021
mbargiel
pushed a commit
to mbargiel/axios
that referenced
this pull request
Jan 27, 2022
* Updating the 'Global axios defaults' README to use a safer example The existing example usage it isn't safe in the general case as it can lead to auth tokens being leaked to 3rd party endpoints by unexpectedly. This change instead gives an example using "axios.defaults.headers.common" to set the User-Agent, which is an equally helpful use-case to document. The 'Custom instance defaults' example just below the 'Global axios defaults' example shows a method to set the 'Authorization' header specific to a given API. I've also updated the variable in the 'Custom instance defaults' code to use a semantically more relevant name within that example. * Revert the example instance name in response to PR request * Reintroduce the Authorization example with a disclaimer about its usage * Update wording nb -> important on usage comment * Remove User-Agent example due to issues with this on Chrome and Safari See axios#1231 Credit @chinesedfan for pointing this out
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If axios is used with multiple domains, the AUTH_TOKEN will be sent to all of them when using the example code:
This PR adds a comment above the example to that effect and points below for an example using Custom instance defaults instead.
The PR also adds an example setting User-Agent, which is another common case for setting
axios.defaults.headers.common
.This is a continuation of #3471 which was previously closed.