Duplicate header keys #19
Comments
|
Thanks for the bug report, you're right, I was overwriting the previous values like a savage. |
|
FYI, to be compatible with the HTTP 1.1 rfc, if multiple header name are sent by the server in the response, the values will be joined in a single header value, separated by a comma |
|
You are exactly right, and I was going to apologize for opening the issue
after I determined that the api I was connecting to is technically at fault
here. Ulfius and Curl are following the spec. I can't expect you to
compensate for everyone else's careless protocol!
That overwrite was inconvenient for me, but it is not incorrect.
…On Thu, Jun 1, 2017 at 3:02 PM, Nicolas Mora ***@***.***> wrote:
FYI, to be compatible with the HTTP 1.1 rfc
<https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2>, if
multiple header name are sent by the server in the response, the values
will be joined in a single header value, separated by a comma ,.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARQOZW_P5o6zO6kELQokau3zhgECHNgBks5r_ybJgaJpZM4Ntf5g>
.
|
|
No, I mean you were right in the first place, there is a bug in the headers management. The HTTP rfc states that:
So in the end, with the patch, Ulfius concatenates header values that have the same (case insensitive) name in a single value, separated by a comma, and in case of multiple header names that are case-insensitive equal but case-sensitive different, the header name will be one of them, but I don't guarantee which one. Can you test if your problem is fixed in the branch 2.1? I'd like to release it soon. |
|
Well, OK.
Yes, I think I can test it. I haven't loaded your new code, but I can. I
didn't write the other end of the API, but I have access to the source and
I run it locally on my own server. What I did to fix it was to slightly
modify the PHP server source so it put one key in the header and one in the
body. It was a cheap shot, actually, but it allowed me to move on. It was a
tough one to find, and I spent too much time on it. Thank the techie gods
for Wireshark.
So, I can definitely put the server back the way it was and dig out an old
version of my standalone test, which was a hack of your example code anyway
(thank you very much). It'll be an after hours thing. Maybe Monday night.
So, Ulfius failed to protect the king, and Uther was murdered by treachery.
Oddly, I'm writing this code for a guy named Arthur. I thought Ulfius was
an odd choice for the name of the software.
…On Fri, Jun 2, 2017 at 9:11 PM, Nicolas Mora ***@***.***> wrote:
No, I mean you were right in the first place, there is a bug in the
headers management.
The HTTP rfc states that:
- You *can* have multiple headers with the same name but it isn't
recommended
- To have multiple values in a single header, you *must* separate them
with a comma ,
- In case of multiple headers with the same name, the name is
case-insensitive
So in the end, with the patch, Ulfius concatenates header values that have
the same (case insensitive) name in a single value, separated by a comma,
and in case of multiple header names that are case-insensitive equal but
case-sensitive different, the header name will be one of them, but I don't
guarantee which one.
Can you test if your problem is fixed in the branch 2.1? I'd like to
release it soon.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARQOZWy1p9RbxwZ1szaOnTVlodhXs_U9ks5sAHqQgaJpZM4Ntf5g>
.
|
|
Hi, Actually I've ran all my tests set and the branch 2.1 passed them all, so I've released it earlier today. This is now in the master branch. All you have to do now is getting the last code and rebuild it. It should be fine. Let me know if you have any other problems with the fix. And yeah, ulfius was one of the knights of the round table. There is no allegory or meaning of the name, rather than it's my tradition to name my projects among the Arthurian legend. Although some of my other projects try to have a meaningful name somehow, like glewlwyd that is my oauth2 server and was the gatekeeper at Camelot, ulfius when I named it was named like that for now specific reason... |
|
Hi @pluto-skaalhelarsen , Any update on this bug? |
|
Sorry, I've been fighting a mysterious bug all week. We are having a
meeting about all this code tomorrow and will conference call with the guy
who wrote the PHP for the API server I'm communicating with. I'm thinking
that everyone will want to go back to using the redundant headers, so it
won't be just a brief test with me hacking the old code back in. It would
be incorporating a test of your new code into our mainstream development,
so this redundant header bugfix of yours would really get some traffic.
…On Wed, Jun 7, 2017 at 7:44 AM, Nicolas Mora ***@***.***> wrote:
Hi @pluto-skaalhelarsen <https://github.com/pluto-skaalhelarsen> ,
Any update on this bug?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARQOZZyoSSc6nIGYJUcNHPSXrVwbhfhLks5sBqlXgaJpZM4Ntf5g>
.
|
|
I'm not sure I understand everything you say but OK :) |
|
Not being understood is one of the major disadvantages of being crazy, or
possibly the primary symptom. Oh, well...
I was hoping that we at the meeting would agree to put your new code into
our main development branch and revert to the previous server API that
created this duplicate header key issue in the first place. That would mean
that your new code would be exercised a lot more than just a brief test.
Anyway, I installed your new version 2.1 from the git repo and tried to
recompile my standalone test code, then realized that there were structural
changes that I had to make. I had been using static autoinitializations of
the query structures, much like you had in the 1.0 version examples. So, a
few simple changes and it compiled fine.
When I tried to link it, however, I got several messages indicating that
your .so.2.1 was not finding the nstrdup(), etc. functions. Ok, I did an
update on my Rasbian Jesse (Debian) and it linked fine. Apparently ulfius
links against a shared library that needs the current version.
When I ran my standalone test that had previously succeeded, I got no body
in the response where it was expected. Thinking I may have a wrong version
of my own standalone test, I made a fresh copy from my backups and compiled
and linked it against my ulfius 1.0 directory for the ulfius.h and the
ulfius.so. This time I got essentially the same link errors complaining
that ulfius.so couldn't find nstrdup(), etc.
I get essentially identical messages with various combinations. The clean
link is with my updated code (no autoinit of the ulfius structures) with
your updated code (v. 2.1) and my updated Raspbian libraries. Or I can go
back to a clean Raspbian Jesse install with your 1.0 code on it and compile
and link as I previously did. My standalone test works fine and returns the
response body correctly.
Here's an example:
make
gcc -o FfClient FfClient.o -lc -ljansson -lyder -lorcania
-L/home/pi/LibraryCode/ulfius1.0/src -lulfius
FfClient.o: In function `main':
/home/pi/StructRev/ForkAuth/ClaysAuth2.1/FfClient.c:146: undefined
reference to `ulfius_set_json_body_request'
/home/pi/LibraryCode/ulfius1.0/src/libulfius.so: undefined reference to
`nstrcasecmp'
/home/pi/LibraryCode/ulfius1.0/src/libulfius.so: undefined reference to
`nstrndup'
/home/pi/LibraryCode/ulfius1.0/src/libulfius.so: undefined reference to
`nstrcmp'
/home/pi/LibraryCode/ulfius1.0/src/libulfius.so: undefined reference to
`nstrdup'
/home/pi/LibraryCode/ulfius1.0/src/libulfius.so: undefined reference to
`nstrncmp'
collect2: error: ld returned 1 exit status
Makefile:19: recipe for target 'FfClient' failed
make: *** [FfClient] Error 1
Those are the same pesky undefined references that haunt my efforts. Where
does your library get those functions? Is this maybe just a forward
reference in my link line?
So, the decision at the meeting yesterday was to put my ulfius upgrade at a
lower priority and focus my efforts on a simulator I'm writing with no
network interactions. Further testing will be delayed. Sorry...
…On Wed, Jun 7, 2017 at 7:13 PM, Nicolas Mora ***@***.***> wrote:
I'm not sure I understand everything you say but OK :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#19 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARQOZamlOvSOC4dern1CdTJH8tfey97jks5sB0qtgaJpZM4Ntf5g>
.
|
|
Yeah, unfortunately I see now there's some lack in the Not only Ulfius changed in the last branch, but the underlying libraries too: Orcania and Yder. This means recompiling/installing orcania and yder before compiling ulfius. I don't have made packages for ulfius, but for a given version, the Whatever the version you want to use, you must run the command The function So I suggest you reinstall orcania/yder/ulfius from scratch, and try again to recompile your program with the last library. |
Thanks for your previous quick response. I have a problem that may actually be in the curl libraries.
A response to a post returns several header values including to values with the same key. I get a seesion key and an xrsf key with the header key string "Set-Cookie". According to the Wireshark traces both these key value pairs are present in the response, but an enumeration from the response returned by Ulfius appears to overwrite the first value with the second and show only that value in the list. When I call PrintResponse(&Response); from your example code, for example, I get a list with one fewer value than was actually sent, and the value of the repeated key is only the second one that was transmitted.
I followed this problem down into your code, but it looks like you are just setting the curl opts and using the returned list. I'm betting you are more familiar with those calls than I am, so I decided to open this issue.
The text was updated successfully, but these errors were encountered: