Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to flatten MTOM XOP parts back into the main XML document. #17

Merged
merged 1 commit into from May 20, 2015
Merged

Add ability to flatten MTOM XOP parts back into the main XML document. #17

merged 1 commit into from May 20, 2015

Conversation

Connorhd
Copy link
Contributor

MTOM with XOP allows binary data to be included alongside the main document using multipart, this change means savon-multipart will always flatten the data back into the original document (i.e. savon users no longer have to treat XOP documents any differently to normal soap requests).

Not sure if this project is looking for this kind of contribution, but looking at the various multipart issues that have been opened on savon it looks like XOP is a common reason to want multipart support.

@tjarratt
Copy link
Contributor

Thanks for the contribution @Connorhd! I've never had to use XOP with SOAP before, but I can see how this would be a useful feature.

Love the specs you added, they're quite clear and concise. I'm only confused by one thing -- based on my reading, it sounds like the content of the multipart response that is binary is supposed to be Base64 encoded. In the actual code, I would expect there to be a call to Base64.decode64, but instead there's a call to #encode64, which seems incorrect (but I've never had to use XOP, so I'm not entirely sure!)

xop_elements.each do |xop_element|
  href = xop_element.attributes['href'].to_s
  cid = href[4..-1]
  data = @parts.find { |p| p.header['content-id'].to_s == "<#{cid}>" }.body.to_s

  # shouldn't this be decode64 here? vvvv
  xop_element.parent.content = Base64.encode64(data).chomp
end

Other than that, I'm more than happy to merge this.

@Connorhd
Copy link
Contributor Author

The code is actually right, its just a confusing situation. XOP takes base64 encoded binary blobs out of the SOAP document and includes them as a raw binary MIME attachment. Whether or not it does this depends on the size of the binary blob and whether its more efficient to extract it or leave it as base64.

This means the consumer of the SOAP document expects the binary data to be base64 encoded, so this PR ensures the data is always there and encoded consistently.

@tjarratt
Copy link
Contributor

Wouldn't that double base64 encode the contents though?

@Connorhd
Copy link
Contributor Author

Connorhd commented May 1, 2015

So to use the test as an example, a SOAP document might look like this:

<?xml version="1.0" encoding="UTF-8"?><envelope><header></header>
<body><BinaryData>QmluYXJ5RGF0YUdvZXNIZXJl</BinaryData></body></envelope>

The client would expect to base64 decode the element to get at their binary data.

In XOP this binary data can be extracted to a mime attachment (to reduce the overall size of the response by removing base64 overhead). Which looks like

----==_mimepart_4d416ae62fd32_201a8043814c4724
Content-ID: <http://tempuri.org/0>
Content-Transfer-Encoding: 8bit
Content-Type: application/xop+xml;charset=utf-8;type="text/xml"

<?xml version="1.0" encoding="UTF-8"?><envelope ><header></header><body><BinaryData><xop:Include href="cid:http://tempuri.org/1/123456789" xmlns:xop="http://www.w3.org/2004/08/xop/include"/></BinaryData></body></envelope>
----==_mimepart_4d416ae62fd32_201a8043814c4724
Content-ID: <http://tempuri.org/1/123456789>
Content-Transfer-Encoding: binary
Content-Type: application/octet-stream

BinaryDataGoesHere
----==_mimepart_4d416ae62fd32_201a8043814c4724--

This PR means that even if the response is split into multiple mime parts by XOP the savon client can always handle the response as if it wasn't

@tjarratt
Copy link
Contributor

I'm sorry this dropped off my radar for so long @Connorhd. After some time reading through the W3 spec, I'm still extremely convinced that the contents of the mime attachments MUST be assumed to either be binary data, or base64 encoded data.

If you can link me some part of the spec that says the mime attachment body MAY be clear text, and should be base64 encoded by clients, then I will be convinced.

@Connorhd
Copy link
Contributor Author

I'm not suggesting the mime attachment is clear text in my example the string "BinaryDataGoesHere" is meant to represent arbitrary binary data.

http://www.w3.org/TR/2005/REC-xop10-20050125/#xml_infoset_soap_sample is a great example:
First look at "Example: XML Infoset prior to XOP processing" and you'll see the SOAP body contains base64 encoded binary data. Then look at "Example: XML Infoset serialized as a XOP package" and you can see this binary data is now just raw binary data as a mime attachment.

The aim of this PR is to restore all XOP packages to their original state before XOP processing. I.e. if savon-multipart receives the 2nd example it will restore it to be like the first example and the client never has to know XOP was invovled.

@tjarratt
Copy link
Contributor

Oh, I guess I was being quite daft. What you're saying is that clients shouldn't need to care how the bytes are encoded, they just get a Base64 encoded body, and since the XOP packages are never Base64 encoded, this just makes it consistent. Thanks for clearing that up!

tjarratt added a commit that referenced this pull request May 20, 2015
Add ability to flatten MTOM XOP parts back into the main XML document.
@tjarratt tjarratt merged commit 150a860 into savonrb:master May 20, 2015
@tjarratt
Copy link
Contributor

Thanks so much for enlightening me, @Connorhd!

@Connorhd
Copy link
Contributor Author

No problem, thanks for merging!

@Connorhd Connorhd deleted the flatten-xop-data branch May 20, 2015 06:38
@Connorhd Connorhd restored the flatten-xop-data branch May 20, 2015 18:30
@bew bew mentioned this pull request Nov 14, 2018
olleolleolle added a commit that referenced this pull request Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants