Skip to content

Conversation

@Bo98
Copy link
Member

@Bo98 Bo98 commented Oct 21, 2021

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Fixes #12293.

@Bo98 Bo98 added the critical Critical change which should be shipped as soon as possible. label Oct 21, 2021
@BrewTestBot
Copy link
Contributor

Review period skipped due to critical label.

@brphelps
Copy link

I tried this syntax out in my repro case and it does appear to address the issue, thank you 🥇 !

@Bo98
Copy link
Member Author

Bo98 commented Oct 21, 2021

Thanks for testing this out @brphelps! I couldn't reproduce the issue myself on Ubuntu 20.04.

@Bo98 Bo98 merged commit e518ea4 into Homebrew:master Oct 21, 2021
@Bo98 Bo98 deleted the cookie-jar branch October 21, 2021 23:49
@brphelps
Copy link

@mistydemeo @Bo98 -- While I tested this and verified it wasn't overwriting /dev/null, I didn't make sure that the cookie functionality still worked. From what I can tell, doesn't moving to --cookie mean "read from file" instead of "write to file"?

NOTE that the file specified with -b/--cookie is only used as input. No cookies will be stored in the file. To store cookies, use the -c/--cookie-jar option or you could even save the HTTP headers to a file using -D/--dump-header!

I'm not really sure why we were recording cookies to dev-null to begin with-- Do we need the argument at all?

@Bo98
Copy link
Member Author

Bo98 commented Oct 22, 2021

The only thing we need curl to do is to store cookies in memory between redirect requests, basically so curl https://www.crushftp.com/early10/CrushFTP10.zip will work. We don't need any form of persistence beyond that.

Curl does not enable in-memory cookies by default, it only does if you pass a file to --cookie and --cookie-jar. We're not actually interested in reading or writing anything, hence the /dev/null.

@github-actions github-actions bot added the outdated PR was locked due to age label Nov 22, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using Homebrew on linux overwrites /dev/null with a file (from curl wrapper)

4 participants