* [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
@ 2017-02-27 20:14 Daniel P. Berrange
2017-02-27 21:20 ` Eric Blake
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Daniel P. Berrange @ 2017-02-27 20:14 UTC (permalink / raw)
To: qemu-devel; +Cc: Denis V. Lunev, Anton Nefedov, Daniel P. Berrange
According to RFC7230 Section 3.2, header field name is case-insensitive.
Convert the header data into all lowercase before doing string matching
on the headers.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
io/channel-websock.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index a06a4a8..32b7f37 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -33,9 +33,9 @@
#define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
#define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
-#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "Sec-WebSocket-Protocol"
-#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "Sec-WebSocket-Version"
-#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "Sec-WebSocket-Key"
+#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
+#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
+#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "sec-websocket-key"
#define QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY "binary"
@@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
Error **errp)
{
- char *handshake_end;
+ char *handshake_end, *tmp;
ssize_t ret;
/* Typical HTTP headers from novnc are 512 bytes, so limiting
* total header size to 4096 is easily enough. */
@@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
}
}
+ for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
+ *tmp = g_ascii_tolower(*tmp);
+ }
+
if (qio_channel_websock_handshake_process(ioc,
(char *)ioc->encinput.buffer,
- ioc->encinput.offset,
+ handshake_end - (char *)ioc->encinput.buffer,
errp) < 0) {
return -1;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-27 20:14 [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers Daniel P. Berrange
@ 2017-02-27 21:20 ` Eric Blake
2017-02-28 7:29 ` Denis V. Lunev
2017-02-28 10:09 ` Daniel P. Berrange
2017-02-27 21:28 ` no-reply
2017-02-28 10:48 ` Daniel P. Berrange
2 siblings, 2 replies; 10+ messages in thread
From: Eric Blake @ 2017-02-27 21:20 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Denis V. Lunev, Anton Nefedov
[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]
On 02/27/2017 02:14 PM, Daniel P. Berrange wrote:
> According to RFC7230 Section 3.2, header field name is case-insensitive.
> Convert the header data into all lowercase before doing string matching
> on the headers.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> io/channel-websock.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> Error **errp)
> {
> - char *handshake_end;
> + char *handshake_end, *tmp;
> ssize_t ret;
> /* Typical HTTP headers from novnc are 512 bytes, so limiting
> * total header size to 4096 is easily enough. */
Drive-by grammar nit: s/easily/easy/
> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> }
> }
>
> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
> + *tmp = g_ascii_tolower(*tmp);
> + }
> +
> if (qio_channel_websock_handshake_process(ioc,
> (char *)ioc->encinput.buffer,
> - ioc->encinput.offset,
> + handshake_end - (char *)ioc->encinput.buffer,
I'm not sure why this change is here; nothing else in the patch changed
ioc->encinput.offset.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-27 20:14 [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers Daniel P. Berrange
2017-02-27 21:20 ` Eric Blake
@ 2017-02-27 21:28 ` no-reply
2017-02-28 10:48 ` Daniel P. Berrange
2 siblings, 0 replies; 10+ messages in thread
From: no-reply @ 2017-02-27 21:28 UTC (permalink / raw)
To: berrange; +Cc: famz, qemu-devel, den, anton.nefedov
Hi,
This series seems to have some coding style problems. See output below for
more information:
Message-id: 20170227201456.31814-1-berrange@redhat.com
Type: series
Subject: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/cover.1488220970.git.jcody@redhat.com -> patchew/cover.1488220970.git.jcody@redhat.com
Switched to a new branch 'test'
b941173 io: ignore case when matching websockets HTTP headers
=== OUTPUT BEGIN ===
Checking PATCH 1/1: io: ignore case when matching websockets HTTP headers...
ERROR: line over 90 characters
#50: FILE: io/channel-websock.c:258:
+ handshake_end - (char *)ioc->encinput.buffer,
total: 1 errors, 0 warnings, 34 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-27 21:20 ` Eric Blake
@ 2017-02-28 7:29 ` Denis V. Lunev
2017-02-28 10:09 ` Daniel P. Berrange
1 sibling, 0 replies; 10+ messages in thread
From: Denis V. Lunev @ 2017-02-28 7:29 UTC (permalink / raw)
To: Eric Blake, Daniel P. Berrange, qemu-devel; +Cc: Anton Nefedov
On 02/28/2017 12:20 AM, Eric Blake wrote:
> On 02/27/2017 02:14 PM, Daniel P. Berrange wrote:
>> According to RFC7230 Section 3.2, header field name is case-insensitive.
>> Convert the header data into all lowercase before doing string matching
>> on the headers.
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>> io/channel-websock.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>> Error **errp)
>> {
>> - char *handshake_end;
>> + char *handshake_end, *tmp;
>> ssize_t ret;
>> /* Typical HTTP headers from novnc are 512 bytes, so limiting
>> * total header size to 4096 is easily enough. */
> Drive-by grammar nit: s/easily/easy/
>
>> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>> }
>> }
>>
>> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
>> + *tmp = g_ascii_tolower(*tmp);
>> + }
>> +
>> if (qio_channel_websock_handshake_process(ioc,
>> (char *)ioc->encinput.buffer,
>> - ioc->encinput.offset,
>> + handshake_end - (char *)ioc->encinput.buffer,
> I'm not sure why this change is here; nothing else in the patch changed
> ioc->encinput.offset.
>
yep. The rest seems fine to me.
Usage of g_ascii_strdown () seems overkill now. We do not need
the header anymore after this call.
Den
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-27 21:20 ` Eric Blake
2017-02-28 7:29 ` Denis V. Lunev
@ 2017-02-28 10:09 ` Daniel P. Berrange
2017-02-28 10:12 ` Denis V. Lunev
1 sibling, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-02-28 10:09 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, Denis V. Lunev, Anton Nefedov
On Mon, Feb 27, 2017 at 03:20:37PM -0600, Eric Blake wrote:
> On 02/27/2017 02:14 PM, Daniel P. Berrange wrote:
> > According to RFC7230 Section 3.2, header field name is case-insensitive.
> > Convert the header data into all lowercase before doing string matching
> > on the headers.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> > io/channel-websock.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
>
> > @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
> > static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> > Error **errp)
> > {
> > - char *handshake_end;
> > + char *handshake_end, *tmp;
> > ssize_t ret;
> > /* Typical HTTP headers from novnc are 512 bytes, so limiting
> > * total header size to 4096 is easily enough. */
>
> Drive-by grammar nit: s/easily/easy/
>
> > @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> > }
> > }
> >
> > + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
> > + *tmp = g_ascii_tolower(*tmp);
> > + }
> > +
> > if (qio_channel_websock_handshake_process(ioc,
> > (char *)ioc->encinput.buffer,
> > - ioc->encinput.offset,
> > + handshake_end - (char *)ioc->encinput.buffer,
>
> I'm not sure why this change is here; nothing else in the patch changed
> ioc->encinput.offset.
It is a related bug. The ioc->encinput buffer contains upto 4096
bytes, but that data may cover both the header & payload. The
handshake_end pointer refers to the end of the header region.
So if we just pass encinput.offset, then handshake_process()
will be searching part of the payload as well as the header.
This might cause it do match the wrong data. I guess I should
have made that clear in the commit, or even tput it in a
separate commit
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-28 10:09 ` Daniel P. Berrange
@ 2017-02-28 10:12 ` Denis V. Lunev
0 siblings, 0 replies; 10+ messages in thread
From: Denis V. Lunev @ 2017-02-28 10:12 UTC (permalink / raw)
To: Daniel P. Berrange, Eric Blake; +Cc: qemu-devel, Anton Nefedov
On 02/28/2017 01:09 PM, Daniel P. Berrange wrote:
> On Mon, Feb 27, 2017 at 03:20:37PM -0600, Eric Blake wrote:
>> On 02/27/2017 02:14 PM, Daniel P. Berrange wrote:
>>> According to RFC7230 Section 3.2, header field name is case-insensitive.
>>> Convert the header data into all lowercase before doing string matching
>>> on the headers.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>> io/channel-websock.c | 14 +++++++++-----
>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>>> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>>> Error **errp)
>>> {
>>> - char *handshake_end;
>>> + char *handshake_end, *tmp;
>>> ssize_t ret;
>>> /* Typical HTTP headers from novnc are 512 bytes, so limiting
>>> * total header size to 4096 is easily enough. */
>> Drive-by grammar nit: s/easily/easy/
>>
>>> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>>> }
>>> }
>>>
>>> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
>>> + *tmp = g_ascii_tolower(*tmp);
>>> + }
>>> +
>>> if (qio_channel_websock_handshake_process(ioc,
>>> (char *)ioc->encinput.buffer,
>>> - ioc->encinput.offset,
>>> + handshake_end - (char *)ioc->encinput.buffer,
>> I'm not sure why this change is here; nothing else in the patch changed
>> ioc->encinput.offset.
> It is a related bug. The ioc->encinput buffer contains upto 4096
> bytes, but that data may cover both the header & payload. The
> handshake_end pointer refers to the end of the header region.
> So if we just pass encinput.offset, then handshake_process()
> will be searching part of the payload as well as the header.
> This might cause it do match the wrong data. I guess I should
> have made that clear in the commit, or even tput it in a
> separate commit
>
> Regards,
> Daniel
with this comment this is obviously correct :)
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-27 20:14 [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers Daniel P. Berrange
2017-02-27 21:20 ` Eric Blake
2017-02-27 21:28 ` no-reply
@ 2017-02-28 10:48 ` Daniel P. Berrange
2017-02-28 10:54 ` Denis V. Lunev
2 siblings, 1 reply; 10+ messages in thread
From: Daniel P. Berrange @ 2017-02-28 10:48 UTC (permalink / raw)
To: qemu-devel; +Cc: Denis V. Lunev, Anton Nefedov
On Mon, Feb 27, 2017 at 08:14:56PM +0000, Daniel P. Berrange wrote:
> According to RFC7230 Section 3.2, header field name is case-insensitive.
> Convert the header data into all lowercase before doing string matching
> on the headers.
>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
> io/channel-websock.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/io/channel-websock.c b/io/channel-websock.c
> index a06a4a8..32b7f37 100644
> --- a/io/channel-websock.c
> +++ b/io/channel-websock.c
> @@ -33,9 +33,9 @@
> #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
> #define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
>
> -#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "Sec-WebSocket-Protocol"
> -#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "Sec-WebSocket-Version"
> -#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "Sec-WebSocket-Key"
> +#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
> +#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
> +#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "sec-websocket-key"
>
> #define QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY "binary"
>
> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> Error **errp)
> {
> - char *handshake_end;
> + char *handshake_end, *tmp;
> ssize_t ret;
> /* Typical HTTP headers from novnc are 512 bytes, so limiting
> * total header size to 4096 is easily enough. */
> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> }
> }
>
> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
> + *tmp = g_ascii_tolower(*tmp);
> + }
> +
self-nack - this does not in fact work - while it is fine to lowercase
the header keys, we must not touch the header values as some data is
case-sensitive
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-28 10:48 ` Daniel P. Berrange
@ 2017-02-28 10:54 ` Denis V. Lunev
2017-02-28 10:58 ` Denis V. Lunev
0 siblings, 1 reply; 10+ messages in thread
From: Denis V. Lunev @ 2017-02-28 10:54 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Anton Nefedov
On 02/28/2017 01:48 PM, Daniel P. Berrange wrote:
> On Mon, Feb 27, 2017 at 08:14:56PM +0000, Daniel P. Berrange wrote:
>> According to RFC7230 Section 3.2, header field name is case-insensitive.
>> Convert the header data into all lowercase before doing string matching
>> on the headers.
>>
>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>> ---
>> io/channel-websock.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/io/channel-websock.c b/io/channel-websock.c
>> index a06a4a8..32b7f37 100644
>> --- a/io/channel-websock.c
>> +++ b/io/channel-websock.c
>> @@ -33,9 +33,9 @@
>> #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
>> #define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
>>
>> -#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "Sec-WebSocket-Protocol"
>> -#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "Sec-WebSocket-Version"
>> -#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "Sec-WebSocket-Key"
>> +#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
>> +#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
>> +#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "sec-websocket-key"
>>
>> #define QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY "binary"
>>
>> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>> Error **errp)
>> {
>> - char *handshake_end;
>> + char *handshake_end, *tmp;
>> ssize_t ret;
>> /* Typical HTTP headers from novnc are 512 bytes, so limiting
>> * total header size to 4096 is easily enough. */
>> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>> }
>> }
>>
>> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
>> + *tmp = g_ascii_tolower(*tmp);
>> + }
>> +
> self-nack - this does not in fact work - while it is fine to lowercase
> the header keys, we must not touch the header values as some data is
> case-sensitive
>
> Regards,
> Daniel
g-ascii-tolower() will help
Den
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-28 10:54 ` Denis V. Lunev
@ 2017-02-28 10:58 ` Denis V. Lunev
2017-02-28 11:56 ` Daniel P. Berrange
0 siblings, 1 reply; 10+ messages in thread
From: Denis V. Lunev @ 2017-02-28 10:58 UTC (permalink / raw)
To: Daniel P. Berrange, qemu-devel; +Cc: Anton Nefedov
On 02/28/2017 01:54 PM, Denis V. Lunev wrote:
> On 02/28/2017 01:48 PM, Daniel P. Berrange wrote:
>> On Mon, Feb 27, 2017 at 08:14:56PM +0000, Daniel P. Berrange wrote:
>>> According to RFC7230 Section 3.2, header field name is case-insensitive.
>>> Convert the header data into all lowercase before doing string matching
>>> on the headers.
>>>
>>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
>>> ---
>>> io/channel-websock.c | 14 +++++++++-----
>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/io/channel-websock.c b/io/channel-websock.c
>>> index a06a4a8..32b7f37 100644
>>> --- a/io/channel-websock.c
>>> +++ b/io/channel-websock.c
>>> @@ -33,9 +33,9 @@
>>> #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
>>> #define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
>>>
>>> -#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "Sec-WebSocket-Protocol"
>>> -#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "Sec-WebSocket-Version"
>>> -#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "Sec-WebSocket-Key"
>>> +#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
>>> +#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
>>> +#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "sec-websocket-key"
>>>
>>> #define QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY "binary"
>>>
>>> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
>>> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>>> Error **errp)
>>> {
>>> - char *handshake_end;
>>> + char *handshake_end, *tmp;
>>> ssize_t ret;
>>> /* Typical HTTP headers from novnc are 512 bytes, so limiting
>>> * total header size to 4096 is easily enough. */
>>> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
>>> }
>>> }
>>>
>>> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
>>> + *tmp = g_ascii_tolower(*tmp);
>>> + }
>>> +
>> self-nack - this does not in fact work - while it is fine to lowercase
>> the header keys, we must not touch the header values as some data is
>> case-sensitive
>>
>> Regards,
>> Daniel
> g-ascii-tolower() will help
>
> Den
ah, sorry, wrong copy/paste. I meant 'g_ascii_strdown ()'
Den
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers
2017-02-28 10:58 ` Denis V. Lunev
@ 2017-02-28 11:56 ` Daniel P. Berrange
0 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrange @ 2017-02-28 11:56 UTC (permalink / raw)
To: Denis V. Lunev; +Cc: qemu-devel, Anton Nefedov
On Tue, Feb 28, 2017 at 01:58:38PM +0300, Denis V. Lunev wrote:
> On 02/28/2017 01:54 PM, Denis V. Lunev wrote:
> > On 02/28/2017 01:48 PM, Daniel P. Berrange wrote:
> >> On Mon, Feb 27, 2017 at 08:14:56PM +0000, Daniel P. Berrange wrote:
> >>> According to RFC7230 Section 3.2, header field name is case-insensitive.
> >>> Convert the header data into all lowercase before doing string matching
> >>> on the headers.
> >>>
> >>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> >>> ---
> >>> io/channel-websock.c | 14 +++++++++-----
> >>> 1 file changed, 9 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/io/channel-websock.c b/io/channel-websock.c
> >>> index a06a4a8..32b7f37 100644
> >>> --- a/io/channel-websock.c
> >>> +++ b/io/channel-websock.c
> >>> @@ -33,9 +33,9 @@
> >>> #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
> >>> #define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
> >>>
> >>> -#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "Sec-WebSocket-Protocol"
> >>> -#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "Sec-WebSocket-Version"
> >>> -#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "Sec-WebSocket-Key"
> >>> +#define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
> >>> +#define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
> >>> +#define QIO_CHANNEL_WEBSOCK_HEADER_KEY "sec-websocket-key"
> >>>
> >>> #define QIO_CHANNEL_WEBSOCK_PROTOCOL_BINARY "binary"
> >>>
> >>> @@ -223,7 +223,7 @@ static int qio_channel_websock_handshake_process(QIOChannelWebsock *ioc,
> >>> static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> >>> Error **errp)
> >>> {
> >>> - char *handshake_end;
> >>> + char *handshake_end, *tmp;
> >>> ssize_t ret;
> >>> /* Typical HTTP headers from novnc are 512 bytes, so limiting
> >>> * total header size to 4096 is easily enough. */
> >>> @@ -249,9 +249,13 @@ static int qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
> >>> }
> >>> }
> >>>
> >>> + for (tmp = (char *)ioc->encinput.buffer; tmp < handshake_end; tmp++) {
> >>> + *tmp = g_ascii_tolower(*tmp);
> >>> + }
> >>> +
> >> self-nack - this does not in fact work - while it is fine to lowercase
> >> the header keys, we must not touch the header values as some data is
> >> case-sensitive
> >>
> >> Regards,
> >> Daniel
> > g-ascii-tolower() will help
> >
> > Den
> ah, sorry, wrong copy/paste. I meant 'g_ascii_strdown ()'
That would still lowercase both the key & value part of the headers.
We need to only lowercase the key, ie text between start of line
and first ':'. This requires properly parsing the HTTP header.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-28 19:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-27 20:14 [Qemu-devel] [PATCH] io: ignore case when matching websockets HTTP headers Daniel P. Berrange
2017-02-27 21:20 ` Eric Blake
2017-02-28 7:29 ` Denis V. Lunev
2017-02-28 10:09 ` Daniel P. Berrange
2017-02-28 10:12 ` Denis V. Lunev
2017-02-27 21:28 ` no-reply
2017-02-28 10:48 ` Daniel P. Berrange
2017-02-28 10:54 ` Denis V. Lunev
2017-02-28 10:58 ` Denis V. Lunev
2017-02-28 11:56 ` Daniel P. Berrange
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.