Last Comment Bug 507976 - anchor parameter in HTTP link header ignored
: anchor parameter in HTTP link header ignored
Status: RESOLVED FIXED
: student-project
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Windows XP
: -- normal with 1 vote (vote)
: mozilla7
Assigned To: Julian Reschke
:
:
Mentors:
http://tools.ietf.org/html/rfc5988#se...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-02 23:57 PDT by Julian Reschke
Modified: 2011-08-24 04:38 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
proposed patch, processing the "anchor" parameter according to RFC 5988 and skipping the link when it changes the context to a different resource (3.77 KB, patch)
2011-05-13 04:46 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Splinter Review
check the anchor parameter while processing link header fields (6.72 KB, patch)
2011-05-30 07:35 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Splinter Review
check the anchor parameter while processing link header fields (6.71 KB, patch)
2011-06-03 02:29 PDT, Julian Reschke
bzbarsky: review+
Details | Diff | Splinter Review

Description Julian Reschke 2009-08-02 23:57:48 PDT
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 2009-08-03 00:04:32 PDT
won't fix unless a new RFC or other compelling reason indicates we should support this parameter.
Comment 2 Julian Reschke 2009-08-03 00:25:17 PDT
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 [:bz] (still a bit busy) 2010-02-11 13:58:54 PST
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 2010-02-11 14:10:25 PST
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 2011-05-06 05:01:37 PDT
Test case: http://greenbytes.de/tech/tc/httplink/#simplecssanchr

(and the spec is RFC 5988)
Comment 6 Julian Reschke 2011-05-12 05:33:02 PDT
Seems this needs to be addressed in http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#562
Comment 7 Julian Reschke 2011-05-13 04:46:51 PDT
Created 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
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2011-05-25 11:11:02 PDT
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 2011-05-25 11:54:09 PDT
(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 [:bz] (still a bit busy) 2011-05-25 12:00:37 PDT
> 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 2011-05-25 12:18:30 PDT
I'll spend some time on test cases first, and then come back with a new patch...
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2011-05-25 23:08:05 PDT
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 2011-05-27 06:56:58 PDT
(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 [:bz] (still a bit busy) 2011-05-27 08:08:45 PDT
> 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 2011-05-27 08:12:55 PDT
(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 2011-05-30 07:35:59 PDT
Created attachment 536092 [details] [diff] [review]
check the anchor parameter while processing link header fields
Comment 17 Julian Reschke 2011-05-30 07:39:21 PDT
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 [:bz] (still a bit busy) 2011-06-02 20:37:14 PDT
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 2011-06-03 02:29:03 PDT
Created attachment 537116 [details] [diff] [review]
check the anchor parameter while processing link header fields
Comment 20 Julian Reschke 2011-06-03 02:29:47 PDT
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 [:bz] (still a bit busy) 2011-06-07 12:50:10 PDT
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-06-07 13:26:56 PDT
http://hg.mozilla.org/projects/cedar/rev/c1e3ab99d34d
Comment 24 Trif Andrei-Alin[:AlinT] 2011-08-24 04:27:58 PDT
Could anyone provide a testcase or some steps on how i could reproduce this issue?
Thanks.
Comment 25 Julian Reschke 2011-08-24 04:38:56 PDT
There's a set of tests around

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

Note You need to log in before you can comment on or make changes to this bug.