[PATCH 2/5] wsdapi: Add support for custom elements in SOAP header.

[Owen Rudge]

Signed-off-by: Owen Rudge <orudge(a)codeweavers.com>
---
  dlls/wsdapi/soap.c            | 85 
+++++++++++++++++++++++++++++++++++++++++--
  dlls/wsdapi/tests/discovery.c |  4 +-
  dlls/wsdapi/wsdapi_internal.h |  1 +
  dlls/wsdapi/xml.c             |  2 +-
  4 files changed, 86 insertions(+), 6 deletions(-)

[Huw Davies]

On Thu, Mar 29, 2018 at 11:12:01PM +0100, Owen Rudge wrote:
> +static WSDXML_ELEMENT *duplicate_element(WSDXML_ELEMENT *parent, const WSDXML_ELEMENT *node, struct list *namespaces)

I hate to keep saying it, but this should probably return HRESULT with
the duplicated element being returned in a WSDXML_ELEMENT ** [out]
parameter.  That would mean create_soap_header_xml_elements() should do likewise.
The WSDXMLAddChild calls here in particular are screaming out for error
propagation.


FWIW, the rest of the series looks ok at first glance.

Huw.
> +{
> +    WSDXML_ATTRIBUTE *cur_attribute, *new_attribute, *last_attribute = NULL;
> +    WSDXML_ELEMENT *new_element;
> +    WSDXML_TEXT *text_node;
> +    WSDXML_NODE *cur_node;
> +
> +    /* First record the namespace in the discovered namespaces list */
> +    if (!add_discovered_namespace(namespaces, node->Name->Space))
> +        return NULL;
> +
> +    if (FAILED(WSDXMLBuildAnyForSingleElement(node->Name, NULL, &new_element)))
> +        return NULL;
> +
> +    WSDXMLAddChild(parent, new_element);
> +
> +    /* Duplicate the nodes */
> +    cur_node = node->FirstChild;
> +
> +    while (cur_node != NULL)
> +    {
> +        if (cur_node->Type == ElementType)
> +        {
> +            if (duplicate_element(new_element, (WSDXML_ELEMENT *)cur_node, namespaces) == NULL) goto cleanup;
> +        }
> +        else if (cur_node->Type == TextType)
> +        {
> +            text_node = WSDAllocateLinkedMemory(new_element, sizeof(WSDXML_TEXT));
> +            if (text_node == NULL) goto cleanup;
> +
> +            text_node->Node.Next = NULL;
> +            text_node->Node.Type = TextType;
> +            text_node->Text = duplicate_string(text_node, ((WSDXML_TEXT *)cur_node)->Text);
> +
> +            if (text_node->Text == NULL) goto cleanup;
> +
> +            WSDXMLAddChild(new_element, (WSDXML_ELEMENT *)text_node);
> +        }
> +
> +        cur_node = cur_node->Next;
> +    }
> +
> +    /* Duplicate the attributes */
> +    cur_attribute = node->FirstAttribute;
> +
> +    while (cur_attribute != NULL)
> +    {
> +        if ((cur_attribute->Name->Space != NULL) && (!add_discovered_namespace(namespaces, cur_attribute->Name->Space)))
> +            return NULL;
> +
> +        new_attribute = WSDAllocateLinkedMemory(new_element, sizeof(WSDXML_ATTRIBUTE));
> +        if (new_attribute == NULL) goto cleanup;
> +
> +        new_attribute->Element = new_element;
> +        new_attribute->Name = duplicate_name(new_attribute, cur_attribute->Name);
> +        new_attribute->Value = duplicate_string(new_attribute, cur_attribute->Value);
> +        new_attribute->Next = NULL;
> +
> +        if ((new_attribute->Name == NULL) || (new_attribute->Value == NULL)) goto cleanup;
> +
> +        if (last_attribute == NULL)
> +            new_element->FirstAttribute = new_attribute;
> +        else
> +            last_attribute->Next = new_attribute;
> +
> +        last_attribute = new_attribute;
> +        cur_attribute = cur_attribute->Next;
> +    }
> +
> +    return new_element;
> +
> +cleanup:
> +    WSDXMLCleanupElement(new_element);
> +    return NULL;
> +}
> +
> +static WSDXML_ELEMENT *create_soap_header_xml_elements(IWSDXMLContext *xml_context, WSD_SOAP_HEADER *header,
> +    struct list *discovered_namespaces)
>  {
>      WSDXML_ELEMENT *header_element = NULL, *app_sequence_element = NULL;
>      WSDXML_NAME *header_name = NULL;
> @@ -532,7 +609,9 @@ static WSDXML_ELEMENT *create_soap_header_xml_elements(IWSDXMLContext *xml_conte
>
> /* </d:AppSequence> */
>
> -    /* TODO: Write any headers */
> +    /* Write any headers */
> +    if ((header->AnyHeaders != NULL) && (duplicate_element(header_element, header->AnyHeaders, discovered_namespaces) == NULL))
> +        goto cleanup;
>
> /* </s:Header> */
>
> @@ -592,7 +671,7 @@ static HRESULT create_soap_envelope(IWSDXMLContext *xml_context, WSD_SOAP_HEADER
>      if (FAILED(ret)) goto cleanup;
>
> /* Create the header XML elements */
> -    header_element = create_soap_header_xml_elements(xml_context, header);
> +    header_element = create_soap_header_xml_elements(xml_context, header, discovered_namespaces);
>      if (header_element == NULL) goto cleanup;
>
> /* <s:Envelope> */
> diff --git a/dlls/wsdapi/tests/discovery.c b/dlls/wsdapi/tests/discovery.c
> index 19c35dd133..8cfcc127aa 100644
> --- a/dlls/wsdapi/tests/discovery.c
> +++ b/dlls/wsdapi/tests/discovery.c
> @@ -647,8 +647,8 @@ static void Publish_tests(void)
>      ok(app_sequence_seen == TRUE, "AppSequence not received\n");
>      todo_wine ok(metadata_version_seen == TRUE, "MetadataVersion not received\n");
>      todo_wine ok(messageOK == TRUE, "Hello message metadata not received\n");
> -    todo_wine ok(any_header_seen == TRUE, "Custom header not received\n");
> -    todo_wine ok(wine_ns_seen == TRUE, "Wine namespace not received\n");
> +    ok(any_header_seen == TRUE, "Custom header not received\n");
> +    ok(wine_ns_seen == TRUE, "Wine namespace not received\n");
>
> after_publish_test:
>
> diff --git a/dlls/wsdapi/wsdapi_internal.h b/dlls/wsdapi/wsdapi_internal.h
> index 47dde6b2c9..dd7931c67e 100644
> --- a/dlls/wsdapi/wsdapi_internal.h
> +++ b/dlls/wsdapi/wsdapi_internal.h
> @@ -65,5 +65,6 @@ HRESULT send_hello_message(IWSDiscoveryPublisherImpl *impl, LPCWSTR id, ULONGLON
>  /* xml.c */
>
> LPWSTR duplicate_string(void *parentMemoryBlock, LPCWSTR value);
> +WSDXML_NAME *duplicate_name(void *parentMemoryBlock, WSDXML_NAME *name);
>
> #endif
> diff --git a/dlls/wsdapi/xml.c b/dlls/wsdapi/xml.c
> index fc02d8b2d4..968b21786a 100644
> --- a/dlls/wsdapi/xml.c
> +++ b/dlls/wsdapi/xml.c
> @@ -63,7 +63,7 @@ static WSDXML_NAMESPACE *duplicate_namespace(void *parentMemoryBlock, WSDXML_NAM
>      return newNs;
>  }
>
> -static WSDXML_NAME *duplicate_name(void *parentMemoryBlock, WSDXML_NAME *name)
> +WSDXML_NAME *duplicate_name(void *parentMemoryBlock, WSDXML_NAME *name)
>  {
>      WSDXML_NAME *dup;

>

[Owen Rudge]

On 03/04/2018 11:58, Huw Davies wrote:
> I hate to keep saying it, but this should probably return HRESULT with
> the duplicated element being returned in a WSDXML_ELEMENT ** [out]
> parameter.  That would mean create_soap_header_xml_elements() should do likewise.
> The WSDXMLAddChild calls here in particular are screaming out for error
> propagation.

Thanks, I've resubmitted these patches using HRESULT error propagation 
where practical. I've amended a couple of existing functions in the 
first patch to return HRESULT, as these will be used fairly heavily in 
future patches.

Cheers,

Owen