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
[bug]: func_curl adding duplicate header leaves request in bad format #135
Comments
What does the RFC state for multiple headers of the same name? If not allowed, then I believe the code should be updated to handle that situation. If allowed then the ability should be added to have a user overwrite what already exists instead of having multiple headers. As for your fix, I believe that wouldn't work and would eliminate all configured options. |
there is no method in libcurl to replace in a slist but they say you should free slist after usage |
You're referencing libcurl functionality, but you mentioned Asterisk linked list previously. They're different, and the slist is already freed. The slist is created at request time based on global settings as well as settings stored on the datastore, and then freed afterwards. Initializing the list in acf_curl_helper would cause a memory leak as the various options would not be freed, and it would also go against the existing documentation that the CURLOPT settings persist for all invocations of the CURL dialplan function. |
ok I saw that now, but seems to be an issue withe the Asterisk list then, shouldn't it be init somewhere? |
The problem is isolated to httpheaders because of the code: Line 444 in 91c8866
Thus why I stated before: What does the RFC state for multiple headers of the same name? If not allowed, then I believe the code should be updated to handle that situation. If allowed then the ability should be added to have a user overwrite what already exists instead of having multiple headers. Just wiping everything clean after CURL() completes is not an option because that goes against the documented and existing behavior, so it needs to be determined what the RFC allows for httpheaders in order to determine the best way to fix this. |
so what about each time a header is added we traverse the list if the header is there remove it and add the new one? |
That's why I asked what the RFC states - what is allowed? Are multiple allowed? What are the constraints? |
If you don't know the answer to that or can't determine it, then I can open this issue up as we should certainly do something. There is no time frame on if/when it would get looked into. |
dont know the answer but is clear that if you use it as it is fails in any webservice you call |
I think this is already a known issue. I seem to recall seeing it on JIRA, or independently reported by multiple people.
Multiple HTTP headers with the same name are expressly allowed by RFC 2616, 4.2:
In practice, this is so uncommon that arguably any client that does this is probably taking a risk. Servers may concatenate or outright replace. RFC 7230 3.2.2 is a little clearer on this:
Thus in practice, in a valid request payload, you shouldn't need to send multiple headers with the same name. What was suggested previously regarding this issue was a function that would clear all the existing headers, to avoid such an issue. I think replacing an existing header is probably the most intuitive default otherwise; I can't think of a good reason for sending multiple headers with the same name, based on the specification. |
not sure how you cannot read the image but here it is in text: already submitted the PR ucontactxCLI> originate Local/1@testcurl extension 11111@nada |
I started working on something like this for ASTERISK-30088 but never finished it. I will see if I can find it and get it working. |
A use-case from me: https://community.asterisk.org/t/how-to-remove-httpheader-added-via-curlopt/99194 As duplicate headers are allowed by RFC, shouldn't the title of this bug changed to something like "Add a way to remove CURL HTTP header or reset the list of headers altogether"? |
Severity
Minor
Versions
all
Components/Modules
func_curl
Operating Environment
Ubuntu 22
Frequency of Occurrence
Constant
Issue Description
using func_curl from dialplan and adding a header that already exists duplicates it and leave the request in a bad format
Example
one solution could be use
AST_LIST_HEAD_INIT(list);
in acf_curl_helper after the unlock if this is ok for you I will make a patch hereRelevant log output
No response
Asterisk Issue Guidelines
The text was updated successfully, but these errors were encountered: