gopher: reject CR and LF in the selector

[alhudz]

Repro: gopher://host/1/sel%0d%0aINJECT is sent on the wire as sel\r\nINJECT\r\n, so the INJECT line is smuggled as a second request line.
Cause: gopher_do() URL-decodes the selector with REJECT_ZERO, which only blocks a decoded NUL, leaving CR/LF to split the single-line gopher request.
Fix: reject a decoded CR or LF in gopher_do() after the decode and fail with CURLE_URL_MALFORMAT. A TAB stays allowed as the legitimate type-7 selector/search separator, so test1202 keeps passing. test1609 covers the rejection.

[vszakats]

Unrelated observation: the GOPHER label wasn't automatically applied
by the labeler, though the rule is present for it:

curl/.github/labeler.yml

Lines 240 to 245 in 372401f

	GOPHER:
	- all:
	- changed-files:
	- any-glob-to-all-files: "{\
	lib/gopher*\
	}"

https://github.com/curl/curl/actions/runs/27867115900/job/82472952947

[alhudz]

It's the matcher. The GOPHER rule uses any-glob-to-all-files, so it only fires when every changed file matches lib/gopher*. This PR also adds tests/data/test1609 and the tests/data/Makefile.am hunk, neither of which matches, so the condition fails and the label is skipped. Switching it to any-glob-to-any-file (or widening the glob list the way the FTP rule does) would let it match when the test files come along too. Happy to fold that in here, or leave it as a separate change.

[dfandrich]

That definitely doesn't belong in this PR. The rule is deliberately built this way to avoid false positives. Many PRs will touch several protocols when performing a cleanup of some sort, which would add many useless labels to a fair number of PRs. What we really want is a rule that excludes directories like docs/ and tests/ and ensures that all the *other* files match a particular protocol (like gopher), but I haven't found a way to express that with the tools available.

[vszakats]

That definitely doesn't belong in this PR. The rule is deliberately built this way to avoid false positives. Many PRs will touch several protocols when performing a cleanup of some sort, which would add many useless labels to a fair number of PRs. What we really want is a rule that excludes directories like docs/ and tests/ and ensures that all the other files match a particular protocol (like gopher), but I haven't found a way to express that with the tools available.

No probs, I was just wondering on how it works, and if this is expected.
Thanks for the background and explanation!

[vszakats]

It's the matcher. The GOPHER rule uses any-glob-to-all-files, so it only fires when every changed file matches lib/gopher*. This PR also adds tests/data/test1609 and the tests/data/Makefile.am hunk, neither of which matches, so the condition fails and the label is skipped. Switching it to any-glob-to-any-file (or widening the glob list the way the FTP rule does) would let it match when the test files come along too. Happy to fold that in here, or leave it as a separate change.

No, not necessary, we expect tests/docs updates together with src changes.

[alhudz]

Right, makes sense. I'll leave the labeler as is then and keep this PR to the gopher fix.

[dfandrich]

No, not necessary, we expect tests/docs updates together with src changes.

The problem is that if someone DOES include docs along with a gopher change, the presence of a doc file means that, according to the rule, EVERYTHING isn't about gopher so it doesn't get the tag.

[alhudz]

Right, that's the any-glob-to-all-files (really one-glob-to-all-files) behaviour again: one doc or test file and the all-files condition fails.

For a separate change, I think you can express the "ignore docs/tests, require the rest to be gopher" shape without enumerating doc paths like FTP does, by pairing an any-match with an all-match against a widened glob:

GOPHER:
  - all:
      - changed-files:
          - any-glob-to-any-file: 'lib/gopher*'
          - all-globs-to-all-files: '{lib/gopher*,docs/**,tests/**}'

The any-glob-to-any-file stops it firing on docs/tests-only PRs, and the all-globs-to-all-files lets a gopher change carry docs/tests along without dropping the label. Keeping this PR to the gopher fix though.

[Copilot]

Pull request overview

This PR hardens curl’s gopher implementation against request-smuggling by rejecting URL-decoded CR/LF characters in the gopher selector, returning CURLE_URL_MALFORMAT before anything is sent to the server.

Changes:

• Reject decoded \r / \n in gopher_do() after URL-decoding the selector and fail with CURLE_URL_MALFORMAT.
• Add a new gopher regression test (test1609) covering %0d%0a injection in the selector.
• Register the new test in tests/data/Makefile.am.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
lib/gopher.c	Adds CR/LF rejection in the decoded selector to prevent gopher request line splitting.
tests/data/test1609	Introduces a regression test for URL-encoded CRLF in the gopher selector expecting CURLE_URL_MALFORMAT.
tests/data/Makefile.am	Adds test1609 to the autotools test data list.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bagder]

Thanks!