Closed Bug 309442 Opened 19 years ago Closed 18 years ago

Support SOAPAction header on submission

Categories

(Core Graveyard :: XForms, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: allan, Assigned: allan)

References

()

Details

(Keywords: fixed1.8.0.12, fixed1.8.1.4)

Attachments

(2 files, 5 obsolete files)

The XForms 1.1 WD has some additions to the submission element, so that we can
use it for SOAP/WS. Especially this is needed:
"if the action MIME parameter appears in the mediatype then a SOAPAction HTTP
header is added and given a value equal to the content of the action MIME parameter"
[http://www.w3.org/TR/xforms11/#SOAP]

Having to add a HTTP header is crap, but that's SOAP...

It's not XForms 1.0, I know :(, but it's pretty useful. XForms using Web
Services is a really really good use case. I think we should support it now.
We already have SOAPAction code in mozilla, but I doubt we can reuse it from
XForms.  Not too hard though!
I have no problem with getting this in 1.0, but I would rather that we wait
until it is at least a recommendation.  It would be bad to document that the
feature is there when we ship and then have it change underneath us after we go
out the door.   Of course, same can be said for all of the 1.1 items we may
eventually absorb. 
Attached file Testcase
Attached patch Patch (obsolete) — Splinter Review
Here's a go at it. I haven't tested it much, I _really_ got to go now... argh.
XForms needs a element that allows adding http headers :)
(In reply to comment #2)
> I have no problem with getting this in 1.0, but I would rather that we wait
> until it is at least a recommendation.  It would be bad to document that the
> feature is there when we ship and then have it change underneath us after we go
> out the door.

I can only agree! I'm also a quite undecided... but I really think it's a really
good use case, which is why I tried to implement it.

Maybe we could have an xforms.enable-unstable preference controlling "funky"
things like this? And/or possibly a warning on the console about the use of
unstable/possibly not supported feature(s).
(In reply to comment #5)
> XForms needs a element that allows adding http headers :)

I hate SOAP for adding a specific HTTP header...
Attached patch Patch with full implementation (obsolete) — Splinter Review
Here's a finished patch. I haven't created the property check yet though, but
that's fairly simple.

With the SOAPAction we can do some pretty nifty forms, like looking up postal
code -> town through WS, etc. The Google API is WS too, isn't it?
Attachment #196922 - Attachment is obsolete: true
Attached patch Patch v3 (obsolete) — Splinter Review
Attachment #197162 - Attachment is obsolete: true
(In reply to comment #8)
> Created an attachment (id=197162) [edit]
> Patch with full implementation
> 
> Here's a finished patch. I haven't created the property check yet though, but
> that's fairly simple.
> 
> With the SOAPAction we can do some pretty nifty forms, like looking up postal
> code -> town through WS, etc. The Google API is WS too, isn't it?

It is, but we are not allowed to do cross domain xml retrieval, remember!
Attached patch Patch v4 (obsolete) — Splinter Review
*argh* wrong patch submitted. This has a preference called
"xforms.enableExperimentalFeatures".
Assignee: aaronr → allan
Attachment #197172 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(In reply to comment #10)
> (In reply to comment #8)
> > Created an attachment (id=197162) [edit] [edit]
> > Patch with full implementation
> > 
> > Here's a finished patch. I haven't created the property check yet though, but
> > that's fairly simple.
> > 
> > With the SOAPAction we can do some pretty nifty forms, like looking up postal
> > code -> town through WS, etc. The Google API is WS too, isn't it?
> 
> It is, but we are not allowed to do cross domain xml retrieval, remember!

If you trust the form you can...
(In reply to comment #12)
> (In reply to comment #10)
> > (In reply to comment #8)
> > > Created an attachment (id=197162) [edit] [edit] [edit]
> > > Patch with full implementation
> > > 
> > > Here's a finished patch. I haven't created the property check yet though, but
> > > that's fairly simple.
> > > 
> > > With the SOAPAction we can do some pretty nifty forms, like looking up postal
> > > code -> town through WS, etc. The Google API is WS too, isn't it?
> > 
> > It is, but we are not allowed to do cross domain xml retrieval, remember!
> 
> If you trust the form you can...

We actually have a web service security model that allows the end server to
allow cross domain access, but hooking that up is a different bug probably ;)
(In reply to comment #13)
> We actually have a web service security model that allows the end server to
> allow cross domain access, but hooking that up is a different bug probably ;)

:) We need to look at the entire security aspect anyhow.

But what about having this in "guarded" by a preference?
(In reply to comment #14)
> (In reply to comment #13)
> > We actually have a web service security model that allows the end server to
> > allow cross domain access, but hooking that up is a different bug probably ;)
> 
> :) We need to look at the entire security aspect anyhow.
> 
> But what about having this in "guarded" by a preference?

We have one in submission already, xforms.crossdomain.enabled, off by default.
Comment on attachment 197173 [details] [diff] [review]
Patch v4

So what do you say?
Attachment #197173 - Flags: review?(aaronr)
Comment on attachment 197173 [details] [diff] [review]
Patch v4

I only know a small amount about soap, but from what I know the code looks ok
to me.	You probably need Doron to review it, too.

I'm r-'ing because there is no notification to the user that he is using
unsupported functionality.  At the absolute bare minimum, there needs to be JS
console output.

And this patch is only meant for the trunk, right?
Attachment #197173 - Flags: review?(aaronr) → review-
(In reply to comment #17)
> (From update of attachment 197173 [details] [diff] [review] [edit])
> I only know a small amount about soap, but from what I know the code looks ok
> to me.	You probably need Doron to review it, too.
> 
> I'm r-'ing because there is no notification to the user that he is using
> unsupported functionality.  At the absolute bare minimum, there needs to be JS
> console output.

Ok, with a console output on usage your're ok, with it?

> And this patch is only meant for the trunk, right?

Well... hmmm, for now I guess. But it's really needed, which why I'm pushing
it.... hmmm.
I was really hoping to see this in the preview release. Was working on
incorporating it into a demo too.
:(
-Andrew
(In reply to comment #19)
> I was really hoping to see this in the preview release. Was working on
> incorporating it into a demo too.
> :(
> -Andrew

This is a good example of where I am coming from.  If someone, for example
Andrew, wants to demo to a group of people what the benefits of XForms are and
wanted to also show XForms working via SOAP with some web services, then a trunk
nightly or their own branch build with this patch added, should be fine. 
Because the demo'er is just trying to sell the people on XForms in general. 
However, as soon as it goes into the branch and someone demos it, then they are
in essence saying, look at what you can do in Firefox with the latest release! 
the selling point now isn't XForms, but rather the Firefox XForms extension. 
And I don't think that we want to sell people on the ability of the Firefox
XForms extension w.r.t. SOAP transactions until it is fully flushed out and
finalized by the WG.

Is there any estimate of when the 1.1 spec might reach recommendation?
(In reply to comment #18)
> (In reply to comment #17)
> > (From update of attachment 197173 [details] [diff] [review] [edit] [edit])
> > I only know a small amount about soap, but from what I know the code looks ok
> > to me.	You probably need Doron to review it, too.
> > 
> > I'm r-'ing because there is no notification to the user that he is using
> > unsupported functionality.  At the absolute bare minimum, there needs to be JS
> > console output.
> 
> Ok, with a console output on usage your're ok, with it?
> 
> > And this patch is only meant for the trunk, right?
> 
> Well... hmmm, for now I guess. But it's really needed, which why I'm pushing
> it.... hmmm.


Yep.  Put in some JS Console output and keep the patch only in the trunk and I'm
totally fine with it.
(In reply to comment #20)
> (In reply to comment #19)
> > I was really hoping to see this in the preview release. Was working on
> > incorporating it into a demo too.
> > :(
> > -Andrew
> 
> This is a good example of where I am coming from.  If someone, for example
> Andrew, wants to demo to a group of people what the benefits of XForms are and
> wanted to also show XForms working via SOAP with some web services, then a trunk
> nightly or their own branch build with this patch added, should be fine. 
> Because the demo'er is just trying to sell the people on XForms in general. 
> However, as soon as it goes into the branch and someone demos it, then they are
> in essence saying, look at what you can do in Firefox with the latest release! 
> the selling point now isn't XForms, but rather the Firefox XForms extension. 
> And I don't think that we want to sell people on the ability of the Firefox
> XForms extension w.r.t. SOAP transactions until it is fully flushed out and
> finalized by the WG.

I disagree. I think this one sells, maybe not XForms 1.0, but the future of
XForms. Using SOAP/WS is a major selling point for XForms. This is one of the
areas where we can "grab" some marketshare. If only SOAP hadn't broken the
protocol levels we could probably have supported it in XForms 1.0 "out of the
box" ... grrrr.

> Is there any estimate of when the 1.1 spec might reach recommendation?

No idea.

But let's start with taking this to trunk. I agree with you that it's dangerous
to take in non-spec. features. I'll try to create some WG momentum on this.
Attached patch Patch v4 w/warning (obsolete) — Splinter Review
Attachment #197173 - Attachment is obsolete: true
Attachment #199053 - Flags: review?(aaronr)
actually, this could also be packaged as an xforms extension somehow... but we
have to figure out how to handle extension in general, so let's take this
approach for now.
Comment on attachment 199053 [details] [diff] [review]
Patch v4 w/warning

I think that I missed something previously.  I think that you need to set
mIsSOAPRequest back to PR_FALSE if @ref or @instance changes, don't you?  Looks
like if one/both of these attributes are changed such that we don't enter
SerializeDataXML, then mIsSOAPRequest won't be accurate in SendData.

I'm fine with this patch (with my AttributeSet paranoia fixed) as long as it
stays in the trunk.
Attachment #199053 - Flags: review?(aaronr) → review+
(In reply to comment #25)
> (From update of attachment 199053 [details] [diff] [review] [edit])
> I think that I missed something previously.  I think that you need to set
> mIsSOAPRequest back to PR_FALSE if @ref or @instance changes, don't you?  Looks
> like if one/both of these attributes are changed such that we don't enter
> SerializeDataXML, then mIsSOAPRequest won't be accurate in SendData.

You are absolutely right. Good catch. I'll set it to false in Submit().
Attachment #199053 - Attachment is obsolete: true
Attachment #199146 - Flags: review?(smaug)
Comment on attachment 199146 [details] [diff] [review]
Patch w/"attribute" fix

r=me, but Doron knows more about SOAP.
Attachment #199146 - Flags: review?(smaug)
Attachment #199146 - Flags: review?(doronr)
Attachment #199146 - Flags: review+
Comment on attachment 199146 [details] [diff] [review]
Patch w/"attribute" fix

r=me
Attachment #199146 - Flags: review?(doronr) → review+
Checked in to trunk. I've also set the xf-to-branch whiteboard keyword, or else
this will possibly pass into oblivion.
Whiteboard: xf-to-branch
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Removing the "xf-to-branch", as it makes updating from the sync I just did "a bit annoying". I still think this should go into branches :)

smaug suggested that we wrap the code in #ifdef's instead -- that makes sync'ing easier. I do not know whether it's possible to "#ifdef THIS_IS_TRUNK"? We could "#ifdef DEBUG" it? Then it will only work in debug builds, no matter the branch you are on. How about that?
Whiteboard: xf-to-branch
(In reply to comment #31)
> Removing the "xf-to-branch", as it makes updating from the sync I just did "a
> bit annoying". I still think this should go into branches :)
> 
> smaug suggested that we wrap the code in #ifdef's instead -- that makes
> sync'ing easier. I do not know whether it's possible to "#ifdef THIS_IS_TRUNK"?
> We could "#ifdef DEBUG" it? Then it will only work in debug builds, no matter
> the branch you are on. How about that?
> 

well, if we ever drop support for 1.5.x and stop building on that branch, then we could use #ifndef MOZILLA_1_8_BRANCH
adding status whiteboard keyword xf-to-branch-11 so we can keep track of what we need to move to branch once XForms 1.1 reaches recommendation.
Whiteboard: xf-to-branch-11
Can we please give some consideration to moving this from experimental to the main stream now? The ability to set the SOAPAction header is essential to using XForms to interact with Web Services/SOAs. IMHO, waiting until the XForms 1.1 spec reaches recommendation to permit users to gracefully interact with SOAs from XForms is a disservice to those users. Other processors have given their users this ability already, since the alternatives -- building a proxy to insert the header, or bypassing XForms submission altogether -- are likely a lot more onerous than any change that might be required due to last-call comments on the XForms 1.1 spec.
This seems like a reasonable request.  Is it best to track a move to the 1.8 branch via this bug or a new one?  I see no harm in enabling this by default, I guess it could be guarded with the preference 'xforms.enableExperimentalFeatures' if desired.
As a general comment, I am wholly supportive of draft 1.1 features under a flag.
If the feature is a lot of work, it's good to check with the WG on its volatility, but otherwise, I think it's getting to be that time for test implementations.  The PR is "call for implementations," but so that that time even the hard features should be attempted ;0>
I vote YES to enable this fix to set the SOAPAction header sooner than later.  Adding this fix can ONLY help XForms developers and make the Firefox XForms extension a better overall implementation especially when it comes to invoking web services from XForms. 
The problem is of course that if for some reason XForms 1.1 will be 
changed a bit so that it becomes incompatible with our implementation.
In that case enabling SOAP by default would really harm XForms 
developers, or implementators (us).
But still, I don't have a strong opinion on this.
What are you doing now if you want your XForm to interact with a web service that requires the SOAPAction header?
(In reply to comment #39)
> What are you doing now if you want your XForm to interact with a web service
> that requires the SOAPAction header?
> 

run a trunk build, enable the pref.  I do it almost every day of the week.

Consider this scenario: We enable all the 1.1 work for 0.8.  The XForms WG 'tweaks' something in the SOAP stuff.  Our next release isn't for 2-3 months.  So now everyone is coding to the old behavior.  We release 0.9.  1.1 hasn't reached 'recommendation', yet, and something else changes.  Another 2-3 months before we ship 1.0.  Perhaps 1.1 has reached recommendation by then.  Perhaps not.  At the point where we ship 1.0 we HAVE to support what we ship.  That is the whole idea behind a 1.0 release.  So by the time we've shipped 1.0, we may have forced our users to re-write their SOAP stuff twice and it STILL might not be finalized in the spec, yet.  Then when the 1.1 spec is finalized we'll end up having to support two versions of SOAP submission (the one we shipped in our 1.0 release and the one that the spec finally recommends).

And lets say that we decide to pref protect it, instead.  And we end up shipping 1.0 before the spec reaches recommendation.  Do we ship with the pref?  Seems wrong to me to ship a 'production grade' extension with an experimental features pref.  And seems even worse to pull an oft-used feature right before shipping.

That is why I still vote no.  If someone can convince me that there are reasonable solutions to the above two scenarios, then I'm quite willing to change my vote.
Well, I have to admit my xforms is a bit rusty, but couldn't we trigger the soap action stuff if the document included a mustUnderstand="true" on the submission element or something? I know using the extension mechanism was discussed earlier, but if there was a way that the document could, in a standards compliant way, turn the flag on, I think it would be an ideal compromise.
(In reply to comment #41)
> Well, I have to admit my xforms is a bit rusty, but couldn't we trigger the
> soap action stuff if the document included a mustUnderstand="true" on the
> submission element or something? I know using the extension mechanism was
> discussed earlier, but if there was a way that the document could, in a
> standards compliant way, turn the flag on, I think it would be an ideal
> compromise.
> 

I don't think that you can do anything with the prefs outside of the chrome.  Too easy for a document to be malicious with the prefs.  And you'd still have the problem where the form author would do something like this and forgets about it and all the sudden two months down the road his SOAP stuff stopped working.  That's why I didn't want to put it in FF2 surrounded by the pref.  Too easy to turn it on and forget about it.
I guess my point was to not use a pref at all, but to simply handle the soap submission if and only if mustUnderstand was true on the submission element (and the mediatype or whatever was setup?). I'd be okay with just adding an xforms:mustUnderstand to the soap envelope element or something and having that trigger the soap stuff but it's a little less intuitive in my mind. 

If you still want to have a preference that can be disabled, that would override any checking for mustunderstand, that would be okay, but I think we should default it to true then instead of defaulting to false. 

I think it solves your scenarios better than just not doing anything at all. Sure it's not perfect, but it puts the control in the document authors hands.

Have I mentioned lately that it's really very frustrating that the 1.1 spec still hasn't reach recommendation by now.
-Andrew
(In reply to comment #40)
> (In reply to comment #39)
> > What are you doing now if you want your XForm to interact with a web service
> > that requires the SOAPAction header?
> > 
> run a trunk build, enable the pref.  I do it almost every day of the week.

I can't ask my customers to build their own firefox browser/extension to solve this problem.  What do you recommend to solve this "real-world" problem with the firefox-based tools we have today? Is there a solution other than not allowing xforms:submissions to invoke web services?
(In reply to comment #44)
> (In reply to comment #40)
> > (In reply to comment #39)
> > > What are you doing now if you want your XForm to interact with a web service
> > > that requires the SOAPAction header?
> > > 
> > run a trunk build, enable the pref.  I do it almost every day of the week.
> 
> I can't ask my customers to build their own firefox browser/extension to solve
> this problem.  What do you recommend to solve this "real-world" problem with
> the firefox-based tools we have today? Is there a solution other than not
> allowing xforms:submissions to invoke web services?
> 

Nightlies.  If they grab a trunk nightly right after we ship a preview, they'll have practically the same level of code and same level of stability as our preview, plus our 1.1 work.  The only downside is the the potential for unstable-ness in the corresponding nightly browser that they'd have to download.  But like I said, I use the trunk all the time.  The odds of them getting an unusable nightly is pretty slim, I'd put it at about 1 in 10 or 1 in 15.  And they are already throwing caution to the wind using an experimental feature to begin with.  I don't see why using a nightly would be any different.  Once they download one that works for them, they can stick with it for a month or two until our next preview and then repeat the process.  Of course, by then, I'd hope that 1.1 reached recommendation.
(In reply to comment #45)
The current workarounds (those I am aware of) for submitting SOAP envelopes to a web service from an XForms document are
1)	To not use XForms:submission, instead use the javascript soap client APIS to invoke the web service
2)	Use a proxy server between the submission and the web service server to add the SOAPAction header
3)	Use a nightly build.

Does anyone else have a workaround for adding a SOAPAction header when submitting a SOAP Envelope message from XForms?  I am afraid getting and using nightly builds may  be a bit complex for some customers.
(In reply to comment #40)
> (In reply to comment #39)
> > What are you doing now if you want your XForm to interact with a web service
> > that requires the SOAPAction header?
> > 
> 
> run a trunk build, enable the pref.  I do it almost every day of the week.

My only thought is that more usage and general feedback would help in validation the implementation.  So requiring users to grab nightly+ext, would deter quite a few.

> Consider this scenario: We enable all the 1.1 work for 0.8.  The XForms WG
> 'tweaks' something in the SOAP stuff.  Our next release isn't for 2-3 months. 
> So now everyone is coding to the old behavior.  We release 0.9.  1.1 hasn't
> reached 'recommendation', yet, and something else changes.  Another 2-3 months
> before we ship 1.0.  Perhaps 1.1 has reached recommendation by then.  Perhaps
> not.  At the point where we ship 1.0 we HAVE to support what we ship.  That is
> the whole idea behind a 1.0 release.  So by the time we've shipped 1.0, we may
> have forced our users to re-write their SOAP stuff twice and it STILL might not
> be finalized in the spec, yet.  Then when the 1.1 spec is finalized we'll end
> up having to support two versions of SOAP submission (the one we shipped in our
> 1.0 release and the one that the spec finally recommends).

I understand your concern but since Mozilla XForms is just about the only one that doesn't support it, seems most likely that it won't change and that the community is trumping the spec.

> 
> And lets say that we decide to pref protect it, instead.  And we end up
> shipping 1.0 before the spec reaches recommendation.  Do we ship with the pref?

How about a separate 1.1 experimental/alpha extension?  Then it easy to separate 1.0 production version and 1.1 experimental.  Can guard 1.1 features with #ifdef and not prefs.
Just an update.  We WILL be including 1.1 functionality in our 0.8 release.  Majority rules.
checked into 1.8 branch on 2007-04-12
checked into 1.8.0 branch on 2007-04-16
Whiteboard: xf-to-branch-11
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: