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

[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;
>