Bug 5609 - XSLT document() doesn't handle relative paths
Summary: XSLT document() doesn't handle relative paths
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-11-03 01:39 PST by Alastair Rankine
Modified: 2006-01-06 01:36 PST (History)
1 user (show)

See Also:


Attachments
Zipfile containting sample stylesheet and test document (882 bytes, application/zip)
2005-11-03 01:39 PST, Alastair Rankine
no flags Details
proposed fix (9.48 KB, patch)
2006-01-02 15:53 PST, Alexey Proskuryakov
eric: review-
Details | Formatted Diff | Diff
non-ASCII paths test (5.01 KB, application/zip)
2006-01-02 17:04 PST, Alexey Proskuryakov
no flags Details
updated patch (7.56 KB, patch)
2006-01-02 17:36 PST, Alexey Proskuryakov
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alastair Rankine 2005-11-03 01:39:12 PST
(As posted to http://webkit.opendarwin.org/blog/?p=32#comments)

The document() function in XSLT doesn't handle relative pathnames. Enclosed is a simple test case which 
tries several different methods to extract some information from a document, but only the full path (with 
file:// prefix) works. I'm not sure of the strict legality of the test case, but other XSLT engines (eg xsltproc) 
handle this situation well.
Comment 1 Alastair Rankine 2005-11-03 01:39:44 PST
Created attachment 4578 [details]
Zipfile containting sample stylesheet and test document
Comment 2 Alastair Rankine 2005-11-03 01:41:45 PST
Forgot to mention this is WebKit 416.12, Safari 2.0.2
Comment 3 Alexey Proskuryakov 2006-01-02 15:53:10 PST
Created attachment 5427 [details]
proposed fix
Comment 4 Eric Seidel (no email) 2006-01-02 16:20:44 PST
Comment on attachment 5427 [details]
proposed fix

I talked with ap on irc.  I'm dubious about the ascii() to utf8() change, as it
was my impression that all libxml2 apis which specify const char * take a
latin1 string, those which specify xmlChar * take a utf8 string.  But I could
be wrong.  I asked him to provide a test case for this change.

Darin also pointed out a problem with ap's URL manipulation on irc, so r- this
one.
Comment 5 Darin Adler 2006-01-02 16:21:05 PST
Comment on attachment 5427 [details]
proposed fix

The relative URL resolution should use the KURL constructor that takes a base
URL and a relative string. There should not be the code to check isValid() and
the code that calls setPath.

Otherwise looks good.
Comment 6 Darin Adler 2006-01-02 16:58:12 PST
I think the UTF-8 change is correct, but I'd like to see a test case that fails with ascii() and succeeds with 
utf8() to prove it.
Comment 7 Alexey Proskuryakov 2006-01-02 17:04:22 PST
Created attachment 5430 [details]
non-ASCII paths test

A test with non-ASCII paths. Shows that utf8() is a correct choice for the base
URL, but resources that have non-ASCII names still cannot be loaded. Since
xsltproc has the same problem (and Firefox doesn't), this sounds like a bug in
libxslt.
Comment 8 Alexey Proskuryakov 2006-01-02 17:33:13 PST
Oops, I spoke too fast - this test works with either utf8() or ascii() - because xmlDocPtrForString() gets 
percent-encoded ASCII URLs from its caller.

Now, there may be issues related to which encoding was used to form the URL, but since libxslt seems to 
have problems here, there's probably not much sense in investigating this right now (but I'll file a separate 
bug to track this).
Comment 9 Alexey Proskuryakov 2006-01-02 17:36:25 PST
Created attachment 5432 [details]
updated patch
Comment 10 Eric Seidel (no email) 2006-01-02 17:43:05 PST
Comment on attachment 5432 [details]
updated patch

(const char *)xmlNodeGetBase(((xsltTransformContextPtr)ctxt)->document->doc,
((xsltTransformContextPtr)ctxt)->node)
Could be made slightly clearer using a local variable.

Otherwise it looks totally fine.  I don't have quite enough xlst-fu to
understand your test case, but I trust you.
Comment 11 Alastair Rankine 2006-01-06 01:36:30 PST
Verified against 06-01-2006 nightly build. Test case works nicely, thanks.