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

option: Disallow username in URL #2340

Closed
wants to merge 1 commit into from
Closed

Conversation

zagor
Copy link
Member

@zagor zagor commented Feb 25, 2018

This solves TODO 1.1.

@jay
Copy link
Member

jay commented Feb 26, 2018

This needs a test, it can be fairly straightforward I think, just test the return code in the fail case. See test2044 for a very simple example. (edit: on second thought you'll probably need to start the HTTP server in the test so that you have a valid URL to pass)

CI says add to symbols-in-versions and document in cmdline-opts, and tool_help.c. I would rename the tool switch from --disallow-username to --disallow-username-in-url. It's longer but for someone reading a script it's easier to understand what it does.

I suggest not using TODO: as the area because it reads like you're adding something to the todo. sample alternatives
lib: Add option to disallow usernames in URLs
url: Add option CURLOPT_DISALLOW_USERNAME_IN_URL
etc

@zagor zagor changed the title TODO 1.1: Disallow usernames in URLs option: Disallow username in URL Mar 2, 2018
@bagder
Copy link
Member

bagder commented Mar 3, 2018

It seems the new test case 2074 fails sometimes? Is it because of different build options?

@zagor zagor force-pushed the disallow-username branch 2 times, most recently from 40a144d to ef87b86 Compare April 15, 2018 07:17
@bagder bagder added the URL label May 23, 2018
@bagder
Copy link
Member

bagder commented May 23, 2018

@zagor Any chance you're up for rebasing this and fixing the conflict?

@zagor
Copy link
Member Author

zagor commented May 23, 2018

Absolutely!

@zagor zagor force-pushed the disallow-username branch 2 times, most recently from 9cbb743 to 3784e06 Compare May 30, 2018 07:47
@bagder
Copy link
Member

bagder commented May 31, 2018

Thanks! (slightly edited before I pushed)

@lock lock bot locked as resolved and limited conversation to collaborators Aug 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants