[PATCH 5/7] [v2] wsdapi: Build and write Scopes and XAddrs lists for Hello message.

[Owen Rudge]

Signed-off-by: Owen Rudge 
---
  dlls/wsdapi/soap.c | 55 
++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 55 insertions(+)

[Huw Davies]

On Mon, Apr 23, 2018 at 09:12:23PM +0100, Owen Rudge wrote:
> Signed-off-by: Owen Rudge
>
> dlls/wsdapi/soap.c | 55
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c
> index 911fbd2a27..9ba934e99d 100644
> --- a/dlls/wsdapi/soap.c
> +++ b/dlls/wsdapi/soap.c
> @@ -532,6 +534,33 @@ static HRESULT build_types_list(LPWSTR buffer, size_t buffer_size, const WSD_NAM
>      return S_OK;
>  }
>
> +static HRESULT build_uri_list(LPWSTR buffer, size_t buffer_size, const WSD_URI_LIST *list)
> +{
> - size_t memory_needed = 0, string_len = 0;
> - const WSD_URI_LIST *cur = list;
> - LPWSTR cur_buf_pos = buffer;
> -
> - do
> - {
> -
>    /* Calculate space needed, including trailing space */
> -
>    string_len = lstrlenW(cur->Element);
> -
>    memory_needed = (string_len + 1) * sizeof(WCHAR);

Should be one WCHAR more to allow for the ' ';
> -
> -
>    if (cur_buf_pos + memory_needed > buffer + buffer_size)
> -
>        return E_INVALIDARG;
> -
> -
>    if (cur != list)
> -
>        *cur_buf_pos++ = ' ';
> -
> -
>    memcpy(cur_buf_pos, cur->Element, memory_needed);

And the memcpy size would then need to be (string_len + 1) * sizeof(WCHAR)
> -
>    cur_buf_pos += string_len;
> -
> -
>    cur = cur->Next;
> - } while (cur != NULL);
> -
> - return S_OK;
>
> +}
> -
>
> static HRESULT duplicate_element(WSDXML_ELEMENT *parent, const WSDXML_ELEMENT *node, struct list *namespaces)
>  {
>      WSDXML_ATTRIBUTE *cur_attribute, *new_attribute, *last_attribute = NULL;
> @@ -913,6 +942,32 @@ HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLON
>          if (FAILED(ret)) goto cleanup;
>      }
> - /* wsd:Scopes */
> - if (scopes_list != NULL)
> - {
> -
>    buffer = WSDAllocateLinkedMemory(hello_element, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR));
> -
>    if (buffer == NULL) goto failed;
> -
> -
>    ret = build_uri_list(buffer, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR), scopes_list);
> -
>    if (FAILED(ret)) goto cleanup;
> -
> -
>    ret = add_child_element(impl->xmlContext, hello_element, discoveryNsUri, scopesString, buffer, NULL);
> -
>    if (FAILED(ret)) goto cleanup;
> - }
> -
> - /* wsd:XAddrs */
> - if (xaddrs_list != NULL)
> - {
> -
>    buffer = WSDAllocateLinkedMemory(hello_element, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR));
> -
>    if (buffer == NULL) goto failed;
> -
> -
>    ret = build_uri_list(buffer, WSD_MAX_TEXT_LENGTH * sizeof(WCHAR), xaddrs_list);
> -
>    if (FAILED(ret)) goto cleanup;
> -
> -
>    ret = add_child_element(impl->xmlContext, hello_element, discoveryNsUri, xAddrsString, buffer, NULL);
> -
>    if (FAILED(ret)) goto cleanup;
> - }
> - /* Write any body elements */
>    if (any != NULL)
>    {
>

[Owen Rudge]

Hi Huw,
> > - do
> > - {
> > -
> >    /* Calculate space needed, including trailing space */
> > -
> >    string_len = lstrlenW(cur->Element);
> > -
> >    memory_needed = (string_len + 1) * sizeof(WCHAR);
>
> Should be one WCHAR more to allow for the ' ';

In this case, I think the code is correct - if there is a trailing space, then there won't be a null character after it (as we'll have another URL following the space). If it's the last entry in the list, then instead of a trailing space there will be a null character.
> > -
> >    if (cur_buf_pos + memory_needed > buffer + buffer_size)
> > -
> >        return E_INVALIDARG;
> > -
> > -
> >    if (cur != list)
> > -
> >        *cur_buf_pos++ = ' ';
> > -
> > -
> >    memcpy(cur_buf_pos, cur->Element, memory_needed);
>
> And the memcpy size would then need to be (string_len + 1) * sizeof(WCHAR)

In this case the null character is being copied as part of the memcpy statement, then is being replaced by the space on the following loop iteration (when cur != list).

Thanks,

Owen

[Huw Davies]

On Tue, Apr 24, 2018 at 08:20:27AM +0000, Owen Rudge wrote:
> Hi Huw,
> > > - do
> > > - {
> > > -
> > >    /* Calculate space needed, including trailing space */
> > > -
> > >    string_len = lstrlenW(cur->Element);
> > > -
> > >    memory_needed = (string_len + 1) * sizeof(WCHAR);
> >
> > Should be one WCHAR more to allow for the ' ';
>
> In this case, I think the code is correct - if there is a trailing space, then there won't be a null character after it (as we'll have another URL following the space). If it's the last entry in the list, then instead of a trailing space there will be a null character.
> > > -
> > >    if (cur_buf_pos + memory_needed > buffer + buffer_size)
> > > -
> > >        return E_INVALIDARG;
> > > -
> > > -
> > >    if (cur != list)
> > > -
> > >        *cur_buf_pos++ = ' ';
> > > -
> > > -
> > >    memcpy(cur_buf_pos, cur->Element, memory_needed);
> >
> > And the memcpy size would then need to be (string_len + 1) * sizeof(WCHAR)
>
> In this case the null character is being copied as part of the memcpy statement, then is being replaced by the space on the following loop iteration (when cur != list).

Right, got it.

Signed-off-by: Huw Davies