[PATCH 4/4] wsdapi: Add initial support for reading messages; identify Probe message. - wine-devel - winehq.org

List overview All Threads

[PATCH 4/4] wsdapi: Add initial support for reading messages; identify Probe message.

[PATCH] gdiplus/tests: Use the...

[PATCH 5/5] mstask/tests: Add more...

Owen Rudge

6 Jun 2018 6 Jun '18

8:58 p.m.

Signed-off-by: Owen Rudge orudge@codeweavers.com --- dlls/wsdapi/soap.c | 340 +++++++++++++++++++++++++++++++++++++++++- dlls/wsdapi/tests/discovery.c | 6 +- 2 files changed, 341 insertions(+), 5 deletions(-)

Attachments:

0004-wsdapi-Add-initial-support-for-reading-messages-iden.patch (text/x-patch — 14.2 KB)

Show replies by date

Huw Davies

7 Jun 7 Jun

7:56 a.m.

On Wed, Jun 06, 2018 at 09:58:52PM +0100, Owen Rudge wrote:

Signed-off-by: Owen Rudge orudge@codeweavers.com

dlls/wsdapi/soap.c | 340 +++++++++++++++++++++++++++++++++++++++++- dlls/wsdapi/tests/discovery.c | 6 +- 2 files changed, 341 insertions(+), 5 deletions(-)

diff --git a/dlls/wsdapi/soap.c b/dlls/wsdapi/soap.c index 4e191a5c82..2bc974f111 100644 --- a/dlls/wsdapi/soap.c +++ b/dlls/wsdapi/soap.c @@ -45,6 +45,13 @@ static const WCHAR actionHello[] = { 'd','i','s','c','o','v','e','r','y','/', 'H','e','l','l','o', 0 };

+static const WCHAR actionProbe[] = {

'h','t','t','p',':','/','/',

's','c','h','e','m','a','s','.','x','m','l','s','o','a','p','.','o','r','g','/',

'w','s','/','2','0','0','5','/','0','4','/',

'd','i','s','c','o','v','e','r','y','/',

'P','r','o','b','e', 0 };

static const WCHAR actionBye[] = { 'h','t','t','p',':','/','/', 's','c','h','e','m','a','s','.','x','m','l','s','o','a','p','.','o','r','g','/', @@ -98,6 +105,27 @@ struct discovered_namespace LPCWSTR uri; };

+static LPWSTR utf8_to_wide(void *parent, const char *utf8_str, int length) +{

int utf8_str_len = 0, chars_needed = 0, bytes_needed = 0;

LPWSTR new_str = NULL;

if (utf8_str == NULL) return NULL;

utf8_str_len = (length < 0) ? lstrlenA(utf8_str) : length;

chars_needed = MultiByteToWideChar(CP_UTF8, 0, utf8_str, utf8_str_len, NULL, 0);

if (chars_needed <= 0) return NULL;

bytes_needed = sizeof(WCHAR) * (chars_needed + 1);

new_str = WSDAllocateLinkedMemory(parent, bytes_needed);

MultiByteToWideChar(CP_UTF8, 0, utf8_str, utf8_str_len, new_str, chars_needed);

new_str[chars_needed] = 0;

return new_str;

+}

static char *wide_to_utf8(LPCWSTR wide_string, int *length) { char *new_string = NULL; @@ -1081,8 +1109,316 @@ cleanup: return ret; }

+static LPWSTR xml_text_to_wide_string(void *parent_memory, WS_XML_TEXT *text) +{
if (text->textType == WS_XML_TEXT_TYPE_UTF8)

{
   WS_XML_UTF8_TEXT *utf8_text = (WS_XML_UTF8_TEXT *) text;
   return utf8_to_wide(parent_memory, (const char *) utf8_text->value.bytes, utf8_text->value.length);
}

else if (text->textType == WS_XML_TEXT_TYPE_UTF16)

{
   WS_XML_UTF16_TEXT *utf_16_text = (WS_XML_UTF16_TEXT *) text;
   return duplicate_string(parent_memory, (LPCWSTR) utf_16_text->bytes);
}

FIXME("Support for text type %d not implemented.\n", text->textType);

return NULL;
+}

+static BOOL move_to_element(WS_XML_READER *reader, const char *element_name, WS_XML_STRING *uri)

return HRESULT

+{

WS_XML_STRING envelope;

BOOL found = FALSE;

envelope.bytes = (BYTE *) element_name;

envelope.length = strlen(element_name);

envelope.dictionary = NULL;

envelope.id = 0;

return SUCCEEDED(WsReadToStartElement(reader, &envelope, uri, &found, NULL)) && found;

+}

+static BOOL ws_element_to_wsdxml_element(WS_XML_READER *reader, IWSDXMLContext *context, WSDXML_ELEMENT *parent_element)

and here.

+{

WSDXML_ATTRIBUTE *cur_wsd_attrib = NULL, *new_wsd_attrib = NULL;
const WS_XML_ELEMENT_NODE *element_node = NULL;
WSDXML_ELEMENT *cur_element = parent_element;
const WS_XML_TEXT_NODE *text_node = NULL;
LPWSTR uri = NULL, element_name = NULL;
WS_XML_STRING *ns_string = NULL;
WS_XML_ATTRIBUTE *attrib = NULL;
WSDXML_ELEMENT *element = NULL;
const WS_XML_NODE *node = NULL;
WSDXML_NAME *name = NULL;
WSDXML_TEXT *text = NULL;
HRESULT ret;
int i;
for (;;)
{
```
   if (cur_element == NULL) break;
```
```
   ret = WsReadNode(reader, NULL);
```
```
   if (FAILED(ret)) goto cleanup;
```

   ret = WsGetReaderNode(reader, &node, NULL);

```
   if (FAILED(ret)) goto cleanup;
```
```
   switch (node->nodeType)
```
```
   {
```
```
       case WS_XML_NODE_TYPE_ELEMENT:
```

           element_node = (const WS_XML_ELEMENT_NODE *) node;

           uri = utf8_to_wide(NULL, (const char *) element_node->ns->bytes, element_node->ns->length);

           if (uri == NULL) goto cleanup;

           /* Link element_name to uri so they will be freed at the same time */

           element_name = utf8_to_wide(uri, (const char *) element_node->localName->bytes,

               element_node->localName->length);

           if (element_name == NULL) goto cleanup;

           if (FAILED(IWSDXMLContext_AddNameToNamespace(context, uri, element_name, &name))) goto cleanup;

```
           WSDFreeLinkedMemory(uri);
```
```
           uri = NULL;
```

           if (FAILED(WSDXMLBuildAnyForSingleElement(name, NULL, &element))) goto cleanup;

           WSDXMLAddChild(cur_element, element);

```
           cur_wsd_attrib = NULL;
```
```
           /* Add attributes */
```

           for (i = 0; i < element_node->attributeCount; i++)

```
           {
```

               attrib = element_node->attributes[i];

               if (attrib->isXmlNs) continue;

               new_wsd_attrib = WSDAllocateLinkedMemory(element, sizeof(WSDXML_ATTRIBUTE));

               if (new_wsd_attrib == NULL) goto cleanup;

```
               ns_string = attrib->ns;
```

               if (ns_string->length == 0) ns_string = element_node->ns;

               uri = utf8_to_wide(NULL, (const char *) ns_string->bytes, ns_string->length);

               if (uri == NULL) goto cleanup;

               /* Link element_name to uri so they will be freed at the same time */

               element_name = utf8_to_wide(uri, (const char *) attrib->localName->bytes, attrib->localName->length);

               if (element_name == NULL) goto cleanup;

               if (FAILED(IWSDXMLContext_AddNameToNamespace(context, uri, element_name, &name))) goto cleanup;

               WSDFreeLinkedMemory(uri);

```
               uri = NULL;
```

               new_wsd_attrib->Value = xml_text_to_wide_string(new_wsd_attrib, attrib->value);

               if (new_wsd_attrib->Value == NULL) goto cleanup;

               new_wsd_attrib->Name = name;

               new_wsd_attrib->Element = cur_element;

               new_wsd_attrib->Next = NULL;

               if (cur_wsd_attrib == NULL)

                   element->FirstAttribute = new_wsd_attrib;

```
               else
```

                   cur_wsd_attrib->Next = new_wsd_attrib;

               cur_wsd_attrib = new_wsd_attrib;

```
           }
```
```
           cur_element = element;
```
```
           break;
```
```
       case WS_XML_NODE_TYPE_TEXT:
```

           text_node = (const WS_XML_TEXT_NODE *) node;

```
           if (cur_element == NULL)
```
```
           {
```

               WARN("No parent element open but encountered text element!\n");

```
               continue;
```
```
           }
```

           if (cur_element->FirstChild != NULL)

```
           {
```

               WARN("Text node encountered but parent already has child!\n");

```
               continue;
```
```
           }
```

           text = WSDAllocateLinkedMemory(element, sizeof(WSDXML_TEXT));

           if (text == NULL) goto cleanup;

           text->Node.Parent = element;

```
           text->Node.Next = NULL;
```
```
           text->Node.Type = TextType;
```

           text->Text = xml_text_to_wide_string(text, text_node->text);

```
           if (text->Text == NULL)
```
```
           {
```

               WARN("Text node returned null string.\n");

               WSDFreeLinkedMemory(text);

```
               continue;
```
```
           }
```

           cur_element->FirstChild = (WSDXML_NODE *) text;

```
           break;
```

       case WS_XML_NODE_TYPE_END_ELEMENT:

           /* Go up a level to the parent element */

           cur_element = cur_element->Node.Parent;

```
           break;
```
```
       default:
```
```
           break;
```
```
   }
```
}
return TRUE;

+cleanup:

/* Free uri and element_name if applicable */
WSDFreeLinkedMemory(uri);
return FALSE;

+}

+static WSDXML_ELEMENT *find_element(WSDXML_ELEMENT *parent, LPCWSTR name, LPCWSTR ns_uri) +{

WSDXML_ELEMENT *cur = (WSDXML_ELEMENT *) parent->FirstChild;
while (cur != NULL)
{

   if ((lstrcmpW(cur->Name->LocalName, name) == 0) && (lstrcmpW(cur->Name->Space->Uri, ns_uri) == 0))

```
       return cur;
```

   cur = (WSDXML_ELEMENT *) cur->Node.Next;

}
return NULL;

+}

int read_message(const char *xml, int xml_length, WSD_SOAP_MESSAGE **out_msg)

This is also looking like a good contender to return HRESULT and take an [out] int *type parameter.

{

/* TODO: Parse and read message */

return MSGTYPE_UNKNOWN;
WSDXML_ELEMENT *envelope = NULL, *header_element, *body_element;

WS_XML_READER_TEXT_ENCODING encoding;

WS_XML_ELEMENT_NODE *envelope_node;

WSD_SOAP_MESSAGE *soap_msg = NULL;

WS_XML_READER_BUFFER_INPUT input;

WS_XML_ATTRIBUTE *attrib = NULL;

IWSDXMLContext *context = NULL;

WS_XML_STRING *soap_uri = NULL;

const WS_XML_NODE *node;

WS_XML_READER *reader;

LPCWSTR value = NULL;

int i, message_type;

LPWSTR uri, prefix;

WS_HEAP *heap;

HRESULT ret;

message_type = MSGTYPE_UNKNOWN;

ret = WsCreateHeap(16384, 4096, NULL, 0, &heap, NULL);

if (FAILED(ret)) goto cleanup;

ret = WsCreateReader(NULL, 0, &reader, NULL);

if (FAILED(ret)) goto cleanup;

encoding.encoding.encodingType = WS_XML_READER_ENCODING_TYPE_TEXT;

encoding.charSet = WS_CHARSET_AUTO;

input.input.inputType = WS_XML_READER_INPUT_TYPE_BUFFER;

input.encodedData = (char *) xml;

input.encodedDataSize = xml_length;

ret = WsSetInput(reader, (WS_XML_READER_ENCODING *) &encoding, (WS_XML_READER_INPUT *) &input, NULL, 0, NULL);

if (FAILED(ret)) goto cleanup;

soap_uri = populate_xml_string(envelopeNsUri);

if (soap_uri == NULL) goto cleanup;

if (!move_to_element(reader, "Envelope", soap_uri)) goto cleanup;

ret = WsGetReaderNode(reader, &node, NULL);

if (FAILED(ret)) goto cleanup;

if (node->nodeType != WS_XML_NODE_TYPE_ELEMENT) goto cleanup;

envelope_node = (WS_XML_ELEMENT_NODE *) node;

if (FAILED(WSDXMLCreateContext(&context))) goto cleanup;

/* Find XML namespaces from the envelope element's attributes */

for (i = 0; i < envelope_node->attributeCount; i++)

{
   attrib = envelope_node->attributes[i];
   if (attrib->isXmlNs)
   {
       uri = utf8_to_wide(NULL, (const char *) attrib->ns->bytes, attrib->ns->length);
       if (uri == NULL) continue;
       prefix = utf8_to_wide(uri, (const char *) attrib->localName->bytes, attrib->localName->length);
       if (prefix == NULL)
       {
           WSDFreeLinkedMemory(uri);
           continue;
       }
       IWSDXMLContext_AddNamespace(context, uri, prefix, NULL);
       WSDFreeLinkedMemory(uri);
   }
}

/* Create the SOAP message to return to the caller */

soap_msg = WSDAllocateLinkedMemory(NULL, sizeof(WSD_SOAP_MESSAGE));

if (soap_msg == NULL) goto cleanup;

ZeroMemory(soap_msg, sizeof(WSD_SOAP_MESSAGE));

envelope = WSDAllocateLinkedMemory(soap_msg, sizeof(WSDXML_ELEMENT));

if (envelope == NULL) goto cleanup;

ZeroMemory(envelope, sizeof(WSDXML_ELEMENT));

if (!ws_element_to_wsdxml_element(reader, context, envelope)) goto cleanup;

/* Find the header element */

header_element = find_element(envelope, headerString, envelopeNsUri);

if (header_element == NULL) goto cleanup;

if (FAILED(WSDXMLGetValueFromAny(addressingNsUri, actionString, (WSDXML_ELEMENT *) header_element->FirstChild,
   &value))) goto cleanup;
soap_msg->Header.Action = duplicate_string(soap_msg, value);

if (soap_msg->Header.Action == NULL) goto cleanup;

if (FAILED(WSDXMLGetValueFromAny(addressingNsUri, toString, (WSDXML_ELEMENT *) header_element->FirstChild,
   &value))) goto cleanup;
soap_msg->Header.To = duplicate_string(soap_msg, value);

if (soap_msg->Header.To == NULL) goto cleanup;

if (FAILED(WSDXMLGetValueFromAny(addressingNsUri, messageIdString, (WSDXML_ELEMENT *) header_element->FirstChild,
   &value))) goto cleanup;
soap_msg->Header.MessageID = duplicate_string(soap_msg, value);

if (soap_msg->Header.MessageID == NULL) goto cleanup;

/* Find the body element */

body_element = find_element(envelope, bodyString, envelopeNsUri);

if (body_element == NULL) goto cleanup;

/* Now figure out which message we've been sent */

if (lstrcmpW(soap_msg->Header.Action, actionProbe) == 0)

{
   /* TODO: Parse the Probe message */
   *out_msg = soap_msg;
   soap_msg = NULL; /* caller will clean this up */
   message_type = MSGTYPE_PROBE;
}
+cleanup:

free_xml_string(soap_uri);

WSDFreeLinkedMemory(soap_msg);

if (context != NULL) IWSDXMLContext_Release(context);

return message_type;

} diff --git a/dlls/wsdapi/tests/discovery.c b/dlls/wsdapi/tests/discovery.c index 96c2ead7f9..0599123a46 100644 --- a/dlls/wsdapi/tests/discovery.c +++ b/dlls/wsdapi/tests/discovery.c @@ -491,7 +491,7 @@ static void verify_wsdxml_any_text(const char *debug_prefix, WSDXML_ELEMENT *any { WSDXML_TEXT *child;

ok(any != NULL, "%s: any == NULL\n", debug_prefix);

todo_wine ok(any != NULL, "%s: any == NULL\n", debug_prefix);

You'll need to have a really good reason for doing this. Can you re-order things so you don't break the tests?

 if (any == NULL) return;

 child = (WSDXML_TEXT *) any->FirstChild;

@@ -534,7 +534,7 @@ static HRESULT WINAPI IWSDiscoveryPublisherNotifyImpl_ProbeHandler(IWSDiscoveryP static const WCHAR extra_info[] = {'E','x','t','r','a','I','n','f','o',0}; WSD_PROBE *probe_msg = (WSD_PROBE *) pSoap->Body;

   ok(pSoap->Body != NULL, "pSoap->Body == NULL\n");

   todo_wine ok(pSoap->Body != NULL, "pSoap->Body == NULL\n");
   ok(pSoap->Header.To != NULL && lstrcmpW(pSoap->Header.To, discoveryTo) == 0,
       "pSoap->Header.To == '%s'\n", wine_dbgstr_w(pSoap->Header.To));
   ok(pSoap->Header.Action != NULL && lstrcmpW(pSoap->Header.Action, actionProbe) == 0,

@@ -939,7 +939,7 @@ after_publish_test: sprintf(probe_message, testProbeMessage, probe_uuid_str);

     ok(send_udp_multicast_of_type(probe_message, strlen(probe_message), AF_INET) == TRUE, "Sending Probe message failed\n");

   todo_wine ok(WaitForSingleObject(probe_event, 2000) == WAIT_OBJECT_0, "Probe message not received\n");

   ok(WaitForSingleObject(probe_event, 2000) == WAIT_OBJECT_0, "Probe message not received\n");

   RpcStringFreeA(&probe_uuid_str);

}

Owen Rudge

8:15 p.m.

On 07/06/2018 08:56, Huw Davies wrote:

...
+static BOOL move_to_element(WS_XML_READER *reader, const char

*element_name, WS_XML_STRING *uri)

return HRESULT

...
+static BOOL ws_element_to_wsdxml_element(WS_XML_READER *reader,

IWSDXMLContext *context, WSDXML_ELEMENT *parent_element)

and here.

...
int read_message(const char *xml, int xml_length, WSD_SOAP_MESSAGE

**out_msg)

This is also looking like a good contender to return HRESULT and take an [out] int *type parameter.

As per the previous patch, the HRESULT is not ultimately used, and I can't see that there's a great benefit in passing it along. I do understand it for the other functions that ultimately return a value to the caller, but these functions are all called by the listener thread, and a simple identification of the message type is all we require.

If you think it would be beneficial, I can modify these functions to return HRESULT, but it would be good to understand the benefits of doing so.

...
- ok(any != NULL, "%s: any == NULL\n", debug_prefix); + todo_wine ok(any != NULL, "%s: any == NULL\n", debug_prefix);

You'll need to have a really good reason for doing this. Can you re-order things so you don't break the tests?

This will be fixed in the next patchset; I didn't want to include any more code in what's already quite a large patch. There's probably an extra 40 or so lines of code needed to avoid this breakage. There's not really a way I can re-order this that I can think of.

If preferred though, I can submit the patch that will fix this as part of this patchset, so the net result once the set is committed involves no extra todos.

Thanks,

Owen

Huw Davies

8 Jun 8 Jun

7:45 a.m.

On Thu, Jun 07, 2018 at 09:15:02PM +0100, Owen Rudge wrote:

On 07/06/2018 08:56, Huw Davies wrote:

...
...
+static BOOL move_to_element(WS_XML_READER *reader, const char

*element_name, WS_XML_STRING *uri)

...
return HRESULT

...
+static BOOL ws_element_to_wsdxml_element(WS_XML_READER *reader,

IWSDXMLContext *context, WSDXML_ELEMENT *parent_element)

...
and here.

...
int read_message(const char *xml, int xml_length, WSD_SOAP_MESSAGE

**out_msg)

...
This is also looking like a good contender to return HRESULT and take an [out] int *type parameter.

As per the previous patch, the HRESULT is not ultimately used, and I can't see that there's a great benefit in passing it along. I do understand it for the other functions that ultimately return a value to the caller, but these functions are all called by the listener thread, and a simple identification of the message type is all we require.

If you think it would be beneficial, I can modify these functions to return HRESULT, but it would be good to understand the benefits of doing so.

It's mainly just for consistency. I don't feel particularly strongly about it in these cases, however in previous patch-sets I let similar things go, only for it later to become apparent that I shouldn't have.

One way to look at it is that conceptually the function has no idea that the caller cannot use the failure status, so it should return it regardless.

...
...
- ok(any != NULL, "%s: any == NULL\n", debug_prefix); + todo_wine ok(any != NULL, "%s: any == NULL\n", debug_prefix);

You'll need to have a really good reason for doing this. Can you re-order things so you don't break the tests?

This will be fixed in the next patchset; I didn't want to include any more code in what's already quite a large patch. There's probably an extra 40 or so lines of code needed to avoid this breakage. There's not really a way I can re-order this that I can think of.

If preferred though, I can submit the patch that will fix this as part of this patchset, so the net result once the set is committed involves no extra todos.

It certainly can't be left like this at the end of the patch-set. It might be ok (since this isn't exactly a core dll) to include the next patch so that the implementation doesn't regress as far as the tests are concerned.

Huw.

Owen Rudge

8:47 a.m.

It's mainly just for consistency. I don't feel particularly strongly about it in these cases, however in previous patch-sets I let similar things go, only for it later to become apparent that I shouldn't have.

Although these functions are not used in my upcoming patches, I suppose it's feasible that one day they could be used by callers that could make use of the result, so I'll look at amending that.

It certainly can't be left like this at the end of the patch-set. It might be ok (since this isn't exactly a core dll) to include the next patch so that the implementation doesn't regress as far as the tests are concerned.

OK, I can include the patch that will fix the TODO in the patchset, which I think would likely be the simplest way of resolving this.

Thanks,

Owen

2402

Age (days ago)

2404

Last active (days ago)

wine-devel@winehq.org

4 comments

3 participants

tags (0)

participants (3)

Huw Davies
Owen Rudge
Owen Rudge