[Spice-devel] [PATCH spice-server] reds: Check link header magic without waiting all header

[Uri Lublin]

Hi Frediano,

I'd replace "all" in subject with "for the whole"

On 01/26/2017 05:54 PM, Frediano Ziglio wrote:
> This allows the connection to early fail in case initial bytes
> are not correct.
> This allows for instance VNC client to graceful fail connecting
> to a spice-server. This happens easily as the two protocols
> share the same range of ports.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> Tested-by: Daniel P. Berrange <berrange at redhat.com>

Ack.

>
> ---
>
> This is well described in
> https://lists.freedesktop.org/archives/spice-devel/2017-January/035201.html
> I'm not sure the comment I wrote is enough or I should copy some
> explanation from the mail.

I think it's enough to add a reference to rhbz#1416692

Uri.

>
> It add an extra read handling but I don't think it's really
> a performance issue as happening only on initial connection.
> ---
>  server/reds.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index 29485a8..40c9485 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2260,12 +2260,6 @@ static void reds_handle_read_header_done(void *opaque)
>      header->minor_version = GUINT32_FROM_LE(header->minor_version);
>      header->size = GUINT32_FROM_LE(header->size);
>
> -    if (header->magic != SPICE_MAGIC) {
> -        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> -        reds_link_free(link);
> -        return;
> -    }
> -
>      if (header->major_version != SPICE_VERSION_MAJOR) {
>          if (header->major_version > 0) {
>              reds_send_link_error(link, SPICE_LINK_ERR_VERSION_MISMATCH);
> @@ -2292,13 +2286,31 @@ static void reds_handle_read_header_done(void *opaque)
>                             link);
>  }
>
> +static void reds_handle_read_magic_done(void *opaque)
> +{
> +    RedLinkInfo *link = (RedLinkInfo *)opaque;
> +    const SpiceLinkHeader *header = &link->link_header;
> +
> +    if (header->magic != SPICE_MAGIC) {
> +        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
> +        reds_link_free(link);
> +        return;
> +    }
> +
> +    reds_stream_async_read(link->stream,
> +                           ((uint8_t *)&link->link_header) + sizeof(header->magic),
> +                           sizeof(SpiceLinkHeader) - sizeof(header->magic),
> +                           reds_handle_read_header_done,
> +                           link);
> +}
> +
>  static void reds_handle_new_link(RedLinkInfo *link)
>  {
>      reds_stream_set_async_error_handler(link->stream, reds_handle_link_error);
>      reds_stream_async_read(link->stream,
>                             (uint8_t *)&link->link_header,
> -                           sizeof(SpiceLinkHeader),
> -                           reds_handle_read_header_done,
> +                           sizeof(link->link_header.magic),
> +                           reds_handle_read_magic_done,
>                             link);
>  }
>
>