8000 Possible regression in handling URI containing carriage return · Issue #16874 · curl/curl · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Possible regression in handling URI containing carriage return #16874

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.

Sign up for GitHub

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

Jump to bottom
8000 Closed
florianernst opened this issue Mar 30, 2025 · 6 comments
Closed

Possible regression in handling URI containing carriage return #16874

florianernst opened this issue Mar 30, 2025 · 6 comments
Assignees
@bagder
Labels
HTTP not-a-curl-bug This is not a bug in curl regression URL

Comments

@florianernst
Copy link

I did this

"Possible" regression, as it is not clear yet whether this is actually a regression in libcurl or whether depending software needs to rethink their implementation / has to adjust their expectations. Filing here nonetheless to avoid this issue falling through the proverbial cracks before the next release. So here we go, please bear with me:

GNU Libmicrohttpd contains a testcurl test suite. With libcurl up to and including 8.12.1 this test suite succeeded, whereas with newer libcurl versions a specific test fails.

This test in question is tricky_url which tests whether the URI base path /one\rtwo gets returned correctly by libmicrohttpd, as tested via libcurl.

I expected the following

While with previous versions of libcurl this tricky_url test succeeded (and still does), with the current development version of curl it now fails with Wrong URI: '/one%0dtwo'.

This was initially noticed in Debian BTS bug#1101232, when testing libmicrohttpd with libcurl 8.13.0~rc2-2 as then present in Debian, and also forwarded to libmicrohttpd MantisBT bug#9664, as of now without any feedback.

I can still reproduce this test failure in libmicrohttpd when testing with current curl master and also with urlapi: normalize retrieved URLs #16841 (I was curious about this branch due the percent-encoding of the path in the above failure message), whereas in the very same build environment but with libcurl 8.12.1 no failure occurs.

What I don't know is whether libmicrohttpd needs to adjust its expectations or whether this is a regression in libcurl, and admittedly the above information is rather limited. So far I haven't managed to extract a minimal test case from libmicrohttpd, and unfortunately time is currently scarce on my side. And I am slightly sitting on the fence here myself, being neither a libmicrohttpd developer nor a libcurl developer but rather just a package maintainer within Debian who was pointed to this test failure, but I will try to extract some more specific information on this as time permits. However, possibly someone can already provide some input based on this rather general description ...

curl/libcurl version

libcurl 8.13.0-DEV

Tested with

operating system

System information (uname -a): Linux 6.1.0-32-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.1.129-1 (2025-03-06) x86_64
Distribution information (/etc/os-release): PRETTY_NAME="Debian GNU/Linux trixie/sid"
@bagder
Copy link
Member
bagder commented Mar 30, 2025

I can't say that I follow this.

Are you saying that you provide a URL with an embedded newline (\n) and you expect libcurl to convert that somehow for you? What's the exact URL you use that somehow makes something go wrong?

@bagder bagder self-assigned this Mar 30, 2025
bagder added a commit that referenced this issue Mar 30, 2025
@bagder
lib1560: try setting path containing LR or CR
535eac9 
Ref: #16874
@bagder bagder mentioned this issue Mar 30, 2025
Closed
@bagder
Copy link
Member
bagder commented Mar 30, 2025

Ok, this is what I think based on my understanding of the libmicrohttpd test and my own experiments:

The test sets a path with an embedded \r (ascii 13).

libcurl 8.12.1 sends that octet "raw" in the HTTP request, causing an invalid request.

libcurl 8.13.0-DEV sends that encoded as %0d, making it a valid request.

I don't think libcurl should send an invalid request. I think that was a libcurl bug. Thus, I suspect the libmicrohttpd test should probably be modified.

@bagder
Copy link
Member
bagder commented Mar 30, 2025

I toyed with this using this sample application:

#include <stdio.h>
#include <curl/curl.h>

#define EXPECTED_URI_BASE_PATH_TRICKY  "/one\rtwo"

int main(void)
{
  CURL *curl;
  int still_running;
  CURLU *url = curl_url();

  if ((CURLUE_OK != curl_url_set (url, CURLUPART_SCHEME, "http", 0)) ||
      (CURLUE_OK != curl_url_set (url, CURLUPART_HOST, "localhost",
                                  CURLU_PATH_AS_IS

        )) ||
      (CURLUE_OK != curl_url_set (url, CURLUPART_PATH,
                                  EXPECTED_URI_BASE_PATH_TRICKY, 0)))
    fprintf(stderr, "error\n");

  curl = curl_easy_init();
  if(curl) {
    curl_easy_setopt(curl, CURLOPT_CURLU, url);
    curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
    curl_easy_perform(curl);
    curl_easy_cleanup(curl);
  }
  return 0;
}

@bagder bagder added the HTTP label Mar 30, 2025
bagder added a commit that referenced this issue Mar 30, 2025
@bagder
lib1560: test set path containing LR or CR
8579cf4 
Ref: #16874
Closes #16875
@bagder bagder added the not-a-curl-bug This is not a bug in curl label Mar 31, 2025
@florianernst
Copy link
Author

Thanks a lot for your analysis and all insights. Yeah, it seems libmicrohttpd indeed needs to adjust its expectations, and there is no regression in libcurl but rather a change toward not sending an invalid request anymore. Thus now closing this issue here.

I will convey this to the libmicrohttpd developers, and I will ensure their testcurl test suite gets some more exposure via Debian CI.

@andreasstieger
Copy link
andreasstieger commented Apr 10, 2025 •
edited
Loading

libmicrohttpd bug is: libmicrohttpd https://bugs.gnunet.org/view.php?id=9664
openSUSE bug: https://bugzilla.opensuse.org/show_bug.cgi?id=1241036

@bagder bagder mentioned this issue Apr 11, 2025
Closed
@Karlson2k
Copy link
Contributor

Gentoo bug: https://bugs.gentoo.org/953520

pps83 pushed a commit to pps83/curl that referenced this issue Apr 26, 2025
@bagder @pps83
lib1560: test set path containing LR or CR
0b63d0e 
Ref: curl#16874
Closes curl#16875
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
HTTP not-a-curl-bug This is not a bug in curl regression URL
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bagder @Karlson2k @andreasstieger @florianernst
0