Anne van Kesteren wrote:
On Sun, 04 May 2008 11:47:13 +0200, Julian Reschke
<[EMAIL PROTECTED]> wrote:
Review of <http://www.w3.org/TR/2008/WD-XMLHttpRequest-20080415/>.
General points:
a) I'm confused about the approach to this document. On the one hand,
we're being told that it can't define anything not already in use (and
that new stuff belongs into XHR2), on the other hand it relies on
HTML5, which is a moving target. It's good that this stuff is being
written down, but if it relies on HTML5, I'd propose to consider other
publication options.
The problem is that concepts such "origin" and determining the encoding
of a text/html stream are not defined anywhere else. It's not really
clear to me what to do about that.
In some cases, it may be possible to copy the current definition. In
other cases, it may be possible just not to depend on it (for instance,
by not specifying encoding sniffing).
b) Algorithms: the spec uses a method to describe algorithms that IMHO
is extremely hard to read (see for instance send() method). This may
be good for implementors, but seems to be bad for everybody else.
Minimally, the lists should be structured for better readability.
Could you elaborate on what kind of change you envision? I'm not sure
how they are not structured right now.
An example would be steps 8..11 in the description of open():
- these steps deal with credentials, and the whole list would be more
readable if each group of steps that belong together would me structured
that way;
- optimally, thing like this shouldn't be expressed as a set of
instructions, but in a declarative way.
c) Structure: It would be nice if Section 4 had more structure. Right
now it's ugly to navigate and refer to.
This is better in XMLHttpRequest Level 2. I rather not revise that
entire section editorially as it might introduce new errors.
But then, it makes a comparison with XHR2 harder. Please reconsider.
2.1 Dependencies
"DOM
A conforming user agent must support some subset of the
functionality defined in DOM Events and DOM Core that this
specification relies upon. [DOM2Events] [DOM3Core]"
That reads a bit strange. Must the subset be non-empty?
Yes, as stated it must be a subset that matches what XMLHttpRequest
requires from the eventing and core specifications.
Then it would be clearer if it said "the subset" instead of "some subset".
2.2 Terminology
"Two URIs are same-origin if after performing scheme-based
normalization on both URIs as described in section 5.3.3 of RFC 3987
the scheme, ihost and port components are identical. If either URI
does not have an ihost component the URIs must not be considered
same-origin. [RFC3987]"
Why are we referring to the IRI spec (RFC3987) when talking about
URIs, as defined RFC3986?
For scheme-bases normalization and ihost. Maybe I should use IRI instead
of URI?
Well, if we're talking about URIs (and I think we do), then we need to
refer to RFC3986 grammar and comparison rules.
Besides that: this may be a non-optimal example unless we can point to
a definition of "HttpOnly cookies". Can we?
I don't believe we can, but since this was put in mostly for HttpOnly
cookies I rather not remove that. I think it will be clear enough for
people reading the document.
So why don't we refer to the specification for httpOnly? Do you consider
it a problem that it's a Microsoft document?
- TRACK??? There's probably a rational for that. If there is, please
include it in the spec.
It's a security issue, as should be clear from the next bullet point.
As TRACK doesn't seem to be documented anywhere, and not implemented in
current IIS versions anymore, I'd really like to see this made a foot
node. The way it's written now is just totally confusing to every reader
who doesn't know the full story around it.
"If the user argument was not omitted and is not null let stored user
be user encoded using the encoding specified in the relevant
authentication scheme or UTF-8 if the scheme fails to specify an
encoding."
Why is XHR talking about the encoding here? Is "stored user" a string
or a byte array?
(same for password)
They're a string (in the API).
When they are a string, then taking about character encoding doesn't
make any sense here.
"If the value argument is null terminate these steps. (Do not raise an
exception.)."
This makes it impossible to set empty headers, which are allowed in
HTTP. Even worse, it silently fails.
Empty headers can be set using the empty string, no? Not raising an
exception is consistent with implementations and I don't think it
matters much as it doesn't have any effect.
Sorry, was reading one thing, but thinking about something else.
Thinking of it, could you please add a clarification that setting to an
empty string is legal, and MUST NOT be ignored? I recall that
Microsoft's original XHR (ActiveX) implementation got that wrong, not
setting the header at all.
"For security reasons, these steps should be terminated if the header
argument case-insensitively matches one of the following headers:
* Accept-Charset
* Accept-Encoding
* Connection
* Content-Length
* Content-Transfer-Encoding
* Date
* Expect
* Host
* Keep-Alive
* Referer
* TE
* Trailer
* Transfer-Encoding
* Upgrade
* Via "
It's unclear why there's a security reason not to allow things like
"Accept-Charset" or "Accept-Encoding". Please explain.
This was done based on implementation feedback. I haven't investigated
what the reasons were for the various headers. If implementors read this
maybe they could chime in and point it out.
Please. And if they don't, please remove all headers for which nobody
can explain why they are in this list.
General comment on "setRequestHeader(header, value), method": the way
it is specified makes it impossible for a client to reliably set
headers. We need a way to either retrieve the current value for
inspection, or a way to reset the header. Or both.
http://lists.w3.org/Archives/Public/public-webapi/2008May/0139.html
Yes, we continue to disagree on this.
"If stored method is GET act as if the data argument is null."
Another case of HTTP/1.1 being profiled. Don't do it.
This was done on request of implementations.
That's IMHO not sufficient reason to do it. Please add a convincing
rational, or leave this to the HTTP WG.
"Serialize data into a namespace well-formed XML document and encoded
using the encoding given by data.inputEncoding, when not null, or
UTF-8 otherwise. Or, if this fails because the Document cannot be
serialized act as if data is null."
Silent failure????
Yes.
Very bad.
Does anybody rely on that? I would be very suprised.
"If no Content-Type header has been set using setRequestHeader()
append a Content-Type header to the list of request headers with a
value of application/xml;charset=charset where charset is the
encoding used to encode the document."
This will result in an invalid Content-Type header if the UA has
initialized the headers with a default (which I think the spec
currently allows; and at least one UA was reported to do). See comment
above about header handling.
Rephrased.
Pointer?
"If the user agent supports HTTP State Management it should persist,
discard and send cookies (as received in the Set-Cookie and
Set-Cookie2 response headers, and sent in the Cookie header) as
applicable. [RFC2965]"
This should probably include a reference to the Set-Cookie (not
Set-Cookie2) spec as well (RFC2109).
I believe it used to do that and it was pointed out that that
specification is not useful in practice and would actually do more harm
than good. I'm not really sure what to do here.
Well, the one that is not used in practice is RFC2965, not RFC2109. That
being said, you probably need to reference both.
"// The following script:
var client = new XMLHttpRequest();
client.open("GET", "test.txt", true);
client.send();
client.onreadystatechange = function() {
if(this.readyState == 3) {
print(this.getAllResponseHeaders());
}
}
// ...should output something similar to the following text:
Date: Sun, 24 Oct 2004 04:58:38 GMT
Server: Apache/1.3.31 (Unix)
Keep-Alive: timeout=15, max=99
Connection: Keep-Alive
Transfer-Encoding: chunked
Content-Type: text/plain; charset=utf-8"
I think examples like this would be more readable (and take less
space) when using the syncr. mode.
I would like to avoid encouraging authors to use the synchronous API.
Disagreed. I think readability and compactness is more important here.
"status of type unsigned short, readonly
On getting, if available, it must return the HTTP status code sent by
the server (typically 200 for a successful request). Otherwise, if not
available, the user agent must raise an INVALID_STATE_ERR exception."
This may be incorrect when the UA caches (304 vs 200).
That's why it says typically.
Hm, no.
When the UA caches, and the server sent 304, the client will potentially
see a 200. This would contradict what *this* paragraph says.
"statusText of type DOMString, readonly
On getting, if available, it must return the HTTP status text
sent by the server (appears after the status code). Otherwise, if not
available, the user agent must raise an INVALID_STATE_ERR exception."
Really? It seems to me that if somebody really implements this,
clients are likely to break. Why not allow an empty string here?
This is what clients have implemented as far as I can tell. Though the
HTTP status text could be the empty string, if that's what you mean...
Does the "if not available" apply to any of the existing
implementations? Why would it be "not available"? Please clarify.
Finally, my main other issue with this spec that it is silent about
the recommended behaviour for unsafe methods, about which RFC2616 says
in Section 9.1.1
(<http://greenbytes.de/tech/webdav/rfc2616.html#rfc.section.9.1.1>):
"Implementors should be aware that the software represents the user in
their interactions over the Internet, and should be careful to allow
the user to be aware of any actions they might take which may have an
unexpected significance to themselves or others.
In particular, the convention has been established that the GET and
HEAD methods SHOULD NOT have the significance of taking an action
other than retrieval. These methods ought to be considered "safe".
This allows user agents to represent other methods, such as POST, PUT
and DELETE, in a special way, so that the user is made aware of the
fact that a possibly unsafe action is being requested."
Thus, allowing a web page to submit a PUT, POST or DELETE request
without user interaction seems to be a very dangerous thing to me, and
the spec should point that out (see also
<http://ietf.osafoundation.org:8080/bugzilla/show_bug.cgi?id=237>).
All requirements from HTTP are taken over unless explicitly stated so I
don't think this is needed.
Well, the spec repeats lots of things specified somewhere else already.
The warning from the HTTP spec is relevant and should appear here, as
XHR is related to UAs, and existing UAs are known to ignore this
security consideration.
BR, Julian
