All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.