507976 - anchor parameter in HTTP link header ignored

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Julian Reschke]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1 

It appears that Firefox ignores the "anchor" parameter (see
http://tools.ietf.org/html/rfc2068#section-19.6.2.4).

What it should do is resolve the anchor (when given) against the request-uri,
and only continue processing it if the resolved URI identifies the same
resource.

Note that RFC 2068 doesn't explain in detail how anchor is to be processed; and that it also has been obsoleted by RFC 2616 which does not describe the link header. IETF draft draft-nottingham-http-link-header will hopefully address that.

Reproducible: Always




(should provide a test case)

Comment 1

[Robert Sayre]

won't fix unless a new RFC or other compelling reason indicates we should support this parameter.

Comment 2

[Julian Reschke]

draft-nottingham-http-link-header is in IETF Last Call, and will likely do not more than clarify the parameter. So I don't think it's the right thing to close the bug.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Relevant spec draft:  http://www.mnot.net/drafts/draft-nottingham-http-link-header-07.txt

I'm still not sure why we should support this, though.  What's the use case?

Comment 4

[Julian Reschke]

The parameter allows resource A to make statements about the links of a different resource B. B might be a sub resource (fragment), or something completely different.

So, in general. this isn't useful for stylesheet links, but it still needs to be processed properly. So if B != A, the link would need to be ignored.

Yes, that's an edge case.

Comment 5

[Julian Reschke]

Test case: http://greenbytes.de/tech/tc/httplink/#simplecssanchr

(and the spec is RFC 5988)

Comment 6

[Julian Reschke]

Seems this needs to be addressed in http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#562

Comment 7

[Julian Reschke]

Attachment: proposed patch, processing the "anchor" parameter according to RFC 5988 and skipping the link when it changes the context to a different resource

Comment 8

[Boris Zbarsky [:bzbarsky]]

I have two questions here:

1)  Why are we using the base URI of the document and not the document URI? Maybe
    it just doesn't matter in this case because they're always equal?
2)  Is it expected that anchor="#foo" will make the Link header only apply if the
    document URI is http://something#foo ?  What about the fact that if a
    resource at http://something is loaded as http://something#foo and the anchor
    URI is http://something the Link won't apply?  Is that purposeful?

Comment 9

[Julian Reschke]

(In reply to comment #8)
> I have two questions here:
> 
> 1)  Why are we using the base URI of the document and not the document URI?
> Maybe
>     it just doesn't matter in this case because they're always equal?

I honestly have no idea. Can you point me at docs that explain the difference?

> 2)  Is it expected that anchor="#foo" will make the Link header only apply
> if the
>     document URI is http://something#foo ?  What about the fact that if a

That's an edge case but I believe the conclusion is correct.

>     resource at http://something is loaded as http://something#foo and the
> anchor
>     URI is http://something the Link won't apply?  Is that purposeful?

No, that sounds like a bug. Do I need to use a different base URI (see above), or strip the fragment first?

...also: I should have noted that it might make sense to move the anchor processing down to ProcessLink(), so that it can be special-cased per link relation. However I wasn't sure about the implications of changing the method signature.

Comment 10

[Boris Zbarsky [:bzbarsky]]

> Can you point me at docs that explain the difference?

The document URI is the URI that was dereferenced to retrieve the document (plus the fragment involved, if any).

The base URI is the URI used as a base for relative URI resolution in the document.  It starts out equal to the document URI in many cases, but is affected by HTML <base> elements, if nothing else.  For documents loaded from http:// URIs this may be the only thing that affects it; not sure.

> No, that sounds like a bug.

It's really no different from the previous case....

> Do I need to use a different base URI 

No, that wouldn't change anything.

> or strip the fragment first?

That really depends on the exact behavior we want here wrt fragments.  Can you describe that behavior?

Moving this check to ProcesLink is fine in general.  The details depend on how it's done.

Comment 11

[Julian Reschke]

I'll spend some time on test cases first, and then come back with a new patch...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 532172 [details] [diff] [review]
proposed patch, processing the "anchor" parameter according to RFC 5988 and skipping the link when it changes the context to a different resource

r- pending that.

Comment 13

[Julian Reschke]

(In reply to comment #8)
> 2)  Is it expected that anchor="#foo" will make the Link header only apply
> if the
>     document URI is http://something#foo ?  What about the fact that if a
>     resource at http://something is loaded as http://something#foo and the
> anchor
>     URI is http://something the Link won't apply?  Is that purposeful?

Thanks for asking the hard questions :-).

The Link header field allows the server to make a statement about a link relation between two resources, one of which being the link target, the other one being the requested resource, potentially modified by the anchor parameter.

The server doesn't know no care about the URI used by the UA; it just sees the HTTP request which does not include the fragment identifier. Thus, the UA should strip the fragment identifier (if present) in the document URI when making the comparison. 

Thus adding a fragment to the anchor parameter to the link header field *should* affect the comparison, while adding a fragment to the resource being navigated to should not.

I've added a set of tests at <http://greenbytes.de/tech/tc/httplink>.

(will work on a new patch soonish)

Comment 14

[Boris Zbarsky [:bzbarsky]]

> Thus adding a fragment to the anchor parameter to the link header field
> *should* affect the comparison

In particular making it always fail, right?  I can live with that, I guess.

Comment 15

[Julian Reschke]

(In reply to comment #14)
> > Thus adding a fragment to the anchor parameter to the link header field
> > *should* affect the comparison
> 
> In particular making it always fail, right?  I can live with that, I guess.

Yup. At least for the purpose of the link relations FF correctly knows about.

I could imagine that in the future, we'd want to collect all link information and make it available to the web page, in which case things might look differently.

Comment 16

[Julian Reschke]

Attachment: check the anchor parameter while processing link header fields

Comment 17

[Julian Reschke]

Comment on attachment 536092 [details] [diff] [review]
check the anchor parameter while processing link header fields

new patch; changes:

- we ignore the fragment identifier on the document URI

- the actual check has moved down to ProcessLink, and has been augmented with a comment about why we do so

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 536092 [details] [diff] [review]
check the anchor parameter while processing link header fields

I think "IsLinkContextToDocument" would be better called "LinkContextIsOurDocument".

GetDocumentURI() won't return null in this code.  No need to check for that.

You should probably check for Equals() returning an error result, on the other hand.

r=me with those issues fixed.

Comment 19

[Julian Reschke]

Attachment: check the anchor parameter while processing link header fields

Comment 20

[Julian Reschke]

Comment on attachment 537116 [details] [diff] [review]
check the anchor parameter while processing link header fields

(applied the proposed changes to the patch)

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 537116 [details] [diff] [review]
check the anchor parameter while processing link header fields

>+  if (! NS_SUCCEEDED(rv)) {

  if (NS_FAILED(rv))

I'll make that change before landing this.

Comment 22

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

http://hg.mozilla.org/projects/cedar/rev/c1e3ab99d34d

Comment 23

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/c1e3ab99d34d

Comment 24

[Trif Andrei-Alin[:AlinT]]

Could anyone provide a testcase or some steps on how i could reproduce this issue?
Thanks.

Comment 25

[Julian Reschke]

There's a set of tests around

  http://greenbytes.de/tech/tc/httplink/#simplecssanchr