- From: Mark Watson <watsonm@netflix.com>
- Date: Tue, 11 Mar 2014 09:56:56 -0700
- To: Richard Barnes <rlb@ipv.sx>
- Cc: Ryan Sleevi <sleevi@google.com>, "public-webcrypto@w3.org" <public-webcrypto@w3.org>
- Message-ID: <CAEnTvdA4NdB9OMoZR3Um2H6c5=N4BOQHqmbv33=gj49az56c4A@mail.gmail.com>
Ryan, The bug for this [1] was titled "Make all method failures asynchronous" and the first comment from Google was "I am in favor of making everything be an asynchronous error!". The rationale for making everything asynchronous is that programers would need to do error handling only in one place. So, this is what I implemented, after sending you a detailed discussion as to what I was going to do and a further description of what I had done. If some errors are synchronous, then - unless we are sure all the synchronous errors never need to be handled in production code - people will need a try ... catch as well as the reject handler on the promise. The developer feedback was that this was what they wanted to avoid. But anyway, I have no problem changing it and the change you suggest is a fairly simple change to the existing text. WebIDL uses the term "type mapping" generically and "convert", linked to the specific procedure, whenever it asks for this kind of conversion to be done. I confess to not being entirely sure about the difference between an EcmaScript object and a WebIDL object. If we use object in the IDL, then what we will have in our asynchronous section is a WebIDL object. I am not sure if "converting" that to a Dictionary subclass is exactly the same as converting the ES object to a Dictionary subclass ? Note that, still, because we do not have polymorphism, we will need the existing structure where we convert to Algorithm in the method procedures but then pass the original object to the algorithm-specific operation so that it can be converted to the appropriate subclass there. ...Mark [1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=24427 On Tue, Mar 11, 2014 at 9:25 AM, Richard Barnes <rlb@ipv.sx> wrote: > > > > On Tue, Mar 11, 2014 at 4:11 PM, Ryan Sleevi <sleevi@google.com> wrote: > >> Mark, >> >> I do not believe we've agreed upon asynchronous for *ALL* WebIDL errors, >> which I have also previously communicated. It certainly is not what WebKit >> or Blink have implemented, and certainly not how I proposed to you that we >> resolve this. >> >> The *only* thing that needs to be asynchronous is Algorithm >> Normalization. >> >> As it reads, I object to the notion that all WebIDL errors are >> asynchronous, and had the WG been given sufficient time to review the spec, >> would have raised this as a LC blocking issue. This is certainly something >> that must block CR, and ideally something that would not have made it to LC. >> >> I agree with Richard that the current text highlights that we've >> functionally done the equivalent of "#define ONE -2", in that our spec does >> not match expectations - of implementors or developers who, in every other >> WebIDL using spec they read, know precisely what to expect. >> >> Richard, >> >> To make actual forward progress on this, do you agree that we can use the >> "any"/"object" type within WebIDL, combined with a strict *synchronous* >> cloning (for lack of a better term, although NOT to be confused with >> structured clone necessarily) of the object prior to returning the promise, >> and then all *future* operations happen on that cloned object. >> > > This is exactly what I was working on implementing this afternoon. FWIW, > my internal WebIDL uses "object". > > As far as verbs: In discussing this with Boris Zbarsky, who knows WebIDL > far better than me, the verb we were using was "coerce". Not sure if that > has any wider usage. The WebIDL spec uses "convert", as in, "An ECMAScript > value V is converted to an IDL dictionary type value...". > <http://www.w3.org/TR/WebIDL/#es-dictionary> > > --Richard > > > >> The goal here is to avoid any manipulation of the object at the point in >> which control is returned to the caller, much in the way that we wish to >> avoid manipulation to ArrayBuffers. >> >> If so, I will attempt to draft text to this effect. >> >> >> On Tue, Mar 11, 2014 at 8:36 AM, Richard Barnes <rlb@ipv.sx> wrote: >> >>> I would really like to avoid re-writing WebIDL code for this spec. That >>> seems like an indication that we've gotten our layering wrong. >>> >>> I'm willing to allow WebIDL binding errors to be synchronous. If we use >>> something like "any" or "object" for Algorithm, then you're really doing >>> something wrong if you cause an error at that level. >>> >>> --Richard >>> >>> >>> On Tue, Mar 11, 2014 at 3:07 PM, Mark Watson <watsonm@netflix.com>wrote: >>> >>>> >>>> >>>> >>>> On Mon, Mar 10, 2014 at 7:28 PM, Ryan Sleevi <sleevi@google.com> wrote: >>>> >>>>> I haven't reviewed the proposed language, but as expressed to you in >>>>> email, I agree it needs to change from Algorithm into a more opaque type to >>>>> support such an action. >>>>> >>>>> I don't think we can use prose to wipe away a normative reference to >>>>> WebIDL. >>>>> >>>> We can do whatever we like, it's a question of whether we *should*. We >>>> can say "in this specification the symbol '5' is used to represent the the >>>> result of the sum '2+2' if we want to, but we probably shouldn't. >>>> >>>> We have said we want all errors to be asynchronous*. This means that >>>> WebIDL type mapping cannot happen for* any of the method parameters*until we say so. It's not even an option to replace all method parameters >>>> with "any", because even type mapping to IDL "any" can throw an exception. >>>> Replacing just some of the types with "any" doesn't help. >>>> >>>> So, we have to say that implementations of WebCrypto must be compliant >>>> to WebIDL *except* that the procedures specified in Section 4.4.7 of >>>> (WebIDL http://www.w3.org/TR/WebIDL/#es-operations) are delayed until >>>> the asynchronous part. There is actually a possibility of a TypeError here >>>> before we get to argument type mapping - in the identification of the >>>> "this" value - so we might need to be more explicit than presently about >>>> which procedures are deferred to the asynchronous part. >>>> >>>> It does seem that, longer term, it would make sense for WebIDL to >>>> incorporate Promises explicitly, so that the delay of these WebIDL steps to >>>> the asynchronous part is placed on a common footing. Then one could expect >>>> generated bindings to be developed for methods with Promise return types >>>> that do this work in the asynchronous part. >>>> >>>> Furthermore, it does seem that Dictionary inheritance is not as useful >>>> as it could be without proper support for polymorphism. WebIDL could >>>> consider having Dictionary IDL types carry with them a reference to the >>>> original object so that downcasts are possible. >>>> >>>> Note that all of the above points to things WebIDL could do that would >>>> - eventually - lead to better development tool support for what we want to >>>> do in WebCrypto. But none of this discussion changes what we want to >>>> achieve in terms of how our methods are called and how they behave. >>>> Implementers are going to have to support the desired behavior "by hand" >>>> until WebIDL has better support for it. >>>> >>>> ...Mark >>>> >>>> * I'm assuming we do not want to revisit this and say that *some* >>>> errors are synchronous: if even one is synchronous then it might as well be >>>> many. >>>> >>>> >>>> >>>> >>>>> On Mar 10, 2014 7:13 PM, "Mark Watson" <watsonm@netflix.com> wrote: >>>>> >>>>>> >>>>>> >>>>>> Sent from my iPhone >>>>>> >>>>>> On Mar 10, 2014, at 6:39 PM, Richard Barnes <rlb@ipv.sx> wrote: >>>>>> >>>>>> So it seems like the editors' inclination (and what's in >>>>>> WebKit/Chrome) is option (1), requiring custom bindings. I can agree with >>>>>> that, and understand the utility of still having the dictionaries around in >>>>>> that case. >>>>>> >>>>>> It still seems like we need to change the use of Algorithm in the >>>>>> SubtleCrypto methods to prevent binding at that method invocation. >>>>>> >>>>>> >>>>>> We have text that says the type mapping doesn't happen on method >>>>>> invocation but when explicitly stated in the asynchronous part. Do you >>>>>> think we need more ? >>>>>> >>>>>> ...Mark >>>>>> >>>>>> >>>>>> --Richard >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Mar 10, 2014 at 11:50 PM, Mark Watson <watsonm@netflix.com>wrote: >>>>>> >>>>>>> Richard, >>>>>>> >>>>>>> Yes, Ryan and I discussed this and I've proposed a solution in the >>>>>>> ED. >>>>>>> >>>>>>> First, since we decided all errors must be asynchronous, we can't >>>>>>> have WebIDL type mapping happening at the usual time. We explicitly note >>>>>>> this and then explicitly specify when to do type mapping. The type mapping >>>>>>> is done in the "normalize to X" algorithm which takes an object and does >>>>>>> WebIDL type mapping to dictionary type X (as well as resolving any aliases, >>>>>>> though we still don't have any of these defined). >>>>>>> >>>>>>> As you say, this mapping discards members not mentioned in X, so >>>>>>> "normalize to Algorithm" leaves you only with name. The individual >>>>>>> algorithm operations call "normalize to Y" where Y is the appropriate >>>>>>> subclass. >>>>>>> >>>>>>> ...Mark >>>>>>> >>>>>>> >>>>>>> On Mon, Mar 10, 2014 at 4:18 PM, Ryan Sleevi <sleevi@google.com>wrote: >>>>>>> >>>>>>>> >>>>>>>> On Mar 10, 2014 4:12 PM, "Richard Barnes" <rlb@ipv.sx> wrote: >>>>>>>> > >>>>>>>> > TL;DR: Usage of Dictionary inheritance for Algorithms is broken. >>>>>>>> Should use WebIDL object type or callback interfaces instead. >>>>>>>> > >>>>>>>> > BACKGROUND: >>>>>>>> > In the current spec, algorithm identifiers and parameters are >>>>>>>> passed to encrypt(), decrypt(), etc. using dictionaries that inherit from >>>>>>>> Algorithm, which is itself a dictionary type. For example: >>>>>>>> > dictionary Algorithm { DOMString name; } >>>>>>>> > dictionary AesCbcParams : Algorithm { CryptoOperationData iv; } >>>>>>>> > typedef (Algorithm or DOMString) AlgorithmIdentifier; >>>>>>>> > Promise<any> encrypt(AlgorithmIdentifier algorithm, ... ) >>>>>>>> > >>>>>>>> > PROBLEM: >>>>>>>> > According to the WebIDL spec for handling of dictionary types, >>>>>>>> the implementation is required to create the dictionary object by copying >>>>>>>> out of the source object the fields specified in the type definition. >>>>>>>> > http://www.w3.org/TR/WebIDL/#es-dictionary >>>>>>>> > >>>>>>>> >>>>>>>> Yes, I raised this privately with Mark when discussing things I >>>>>>>> didn't think were LC ready. >>>>>>>> >>>>>>>> > For WebCrypto, this means that polymorphism via inheritance of >>>>>>>> dictionaries doesn't work. If someone creates a dictionary that matches >>>>>>>> AesCbcParams and passes it to encrypt(), then the WebIDL implementation >>>>>>>> will only store the "name" field, and throw everything else away. Anything >>>>>>>> using the WebIDL object will not be able to see the "iv" parameter. >>>>>>>> > >>>>>>>> >>>>>>>> Did the wording Mark proposed to handle this not address it? >>>>>>>> >>>>>>>> Note the asynchronous behavior of WebIDL conversions that are >>>>>>>> written. >>>>>>>> >>>>>>>> > As far as implementation: I discovered this bug while trying to >>>>>>>> implement using the Firefox WebIDL implementation, so clearly Firefox has >>>>>>>> this problem :) It appears that Chromium is internally treating >>>>>>>> "algorithm" arguments as generic objects (not following the WebIDL spec for >>>>>>>> dictionaries), then using a "parseAlgorithm" method to pull out relevant >>>>>>>> information. >>>>>>>> > >>>>>>>> >>>>>>>> Both Blink and WebKit have implemented this with custom bindings. >>>>>>>> >>>>>>>> > PROPOSED RESOLUTION: >>>>>>>> > It seems to me that we have a couple of options here. >>>>>>>> > >>>>>>>> > 1. "typedef object Algorithm" - Don't have the WebIDL handle any >>>>>>>> of the internal structure of Algorithm objects, but instead lay this out in >>>>>>>> the text of the spec. In this case, there's no inheritance to speak of, >>>>>>>> just lists of what parameters must be present in each use of Algorithm. We >>>>>>>> would need to specify at what point in the process values are extracted >>>>>>>> from the object. >>>>>>>> > >>>>>>>> >>>>>>>> Or just call it "any" >>>>>>>> >>>>>>>> > 2. "callback interface Algorithm" - This would use inheritance >>>>>>>> and have the expected behavior, since callback interfaces keep around a >>>>>>>> copy of the provided JS object. However, there might be some weird >>>>>>>> behaviors if people pass in exotic objects, e.g., side-effects or changing >>>>>>>> values. Also, the spec would need to carefully define exactly when the >>>>>>>> properties are accessed, and all per-spec access to the properties would >>>>>>>> need to be done on the main thread, since it needs to be able to run script. >>>>>>>> > >>>>>>>> >>>>>>>> Strongly opposed to this. Its going to be quite ugly and requires >>>>>>>> painting over all of the ES5/ES6 interactions. >>>>>>>> >>>>>>>> > I have a slight preference for (2). It continues to delegate >>>>>>>> most of the work to WebIDL, and it seems unlikely that people will do >>>>>>>> exotic enough things to cause trouble, given that the way WebCrypto uses >>>>>>>> these objects is fairly straightforward. >>>>>>>> > >>>>>>>> > --Richard >>>>>>>> > >>>>>>>> > >>>>>>>> >>>>>>>> Considering that the WebIDL spec changed in order to support other >>>>>>>> use cases of ours - notably, the Promise<resolve type> notation - have you >>>>>>>> considered approaching the WebIDL WG with this? >>>>>>>> >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>> >>> >> >
Received on Tuesday, 11 March 2014 16:57:26 UTC