Skip to content

Fix csharp-restsharp user agent header issue #170

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

Merged
merged 6 commits into from
Mar 2, 2020

Conversation

ttzztztz
Copy link
Contributor

I fixed issue #158 by changing the C# RestSharp API .

As He says, the

request.AddHeader("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100101 Firefox/72.0");

doesn't work and will be overwritten to a default UA, the official solution is to use

client.UserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100101 Firefox/72.0";

Instead of Add Header.

So I just fix it by checking whether a Header's key equals to User-Agent, If so, output the client.UserAgent = "...", otherwise, output the request.AddHeader(..., ...).

So It will work and fix the issues.

@shreys7 I added an extra unit test for my fix. This is my first time contributing to the open source community, thank you for your guidance!

@shreys7
Copy link
Member

shreys7 commented Feb 24, 2020

Hey @ttzztztz , you need not create a new PR for adding a unit test for the fix. You can add it to your branch and the PR will be updated the latest changes.

@ttzztztz
Copy link
Contributor Author

Hey @ttzztztz , you need not create a new PR for adding a unit test for the fix. You can add it to your branch and the PR will be updated the latest changes.

Oh, Got it ! Thank you for your notice!

shreys7
shreys7 previously approved these changes Feb 27, 2020
@shreys7 shreys7 changed the title Fix issue #158 with unit test Fix csharp-restsharp user agent header issue Feb 28, 2020
@umeshp7
Copy link
Contributor

umeshp7 commented Mar 1, 2020

Re-run tests.
waiting for tests to pass.

@umeshp7
Copy link
Contributor

umeshp7 commented Mar 1, 2020

Copy link
Contributor

@umeshp7 umeshp7 left a comment

Choose a reason for hiding this comment

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

Some lint issues found after running tests needs resolving before this can be merged.

@umeshp7 umeshp7 requested a review from shreys7 March 1, 2020 14:12
@umeshp7 umeshp7 dismissed shreys7’s stale review March 1, 2020 14:13

Tests failing.

@ttzztztz
Copy link
Contributor Author

ttzztztz commented Mar 2, 2020

@shreys7 Hello, I updated my codes and it passed the unit test and lint check. Could you please review it again? Thank you!

@ttzztztz ttzztztz requested a review from shreys7 March 2, 2020 11:09
@shreys7
Copy link
Member

shreys7 commented Mar 2, 2020

Hey @ttzztztz , could you mark @umeshp7's review comments as resolved? It's good to go then.

@ttzztztz ttzztztz requested a review from umeshp7 March 2, 2020 12:26
@ttzztztz
Copy link
Contributor Author

ttzztztz commented Mar 2, 2020

@shreys7 Already marked. Thank you for your approval.

@umeshp7 umeshp7 merged commit 5970d80 into postmanlabs:develop Mar 2, 2020
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.

3 participants