AW: AW: [Patch] 6502392 Invalid relative names for Filer.createResource and Filer.getResource

[Jonathan Gibbons]

Emil,

Comments inline.

-- Jon


Emil Siemes wrote:
> Hi Jon,
>
> RFC 3986 Appendix D describes the additions and modifications vs. 2396.
> Basically 3986 tries to make things clearer. They say there have been 
> a lot of discussions and different interpretations of 2396. (Sounds 
> like JME fragmentation to me ;-)
> A new ABNF was used, IPV6 literals added. URI normalization and 
> comparison was rewritten.
> Clarification of reserved and unreserved characters. Support for an 
> empty path component were added.

OK, thanks for the summary.

> 6791041 & 6791060 seem to be non-public. Will this change soon? I were 
> not able to find them on bugs.sun.com
They should now be available on bugs.sun.com.  There is sometimes a 
short delay in propogating bugs
from the internal bug database to the external site.


> If there is more information available on 6791041 I could look for 
> potential interpretation issues in the java.net.URI implementation vs. 
> 3986 spec.
6791041 provided a case where someone actually tried to have a relative 
path beginning with ".", such as ".foo"
>
> I should have a new isRelativeURI Monday latest. Would be good to know 
> what 6791041 is about by then.
Apparently, there was an attempt in JDK 6 to update URI for 3986, and 
the work had to be reverted
for compatibility issues. This makes me less optimistic that we might 
see any evolution for JDK 7.
As such, I think we should leverage what is in URI as much as possible, 
and apply any necessary
extra work in JavacFileManager -- which is essentially where you are 
heading, I believe.

>
> For the missing FileNotFoundException in JavacFiler I think the 
> clearest solution is to explicitly check for file existence in the 
> method. 
> Something like:
>
> getResource(...
>
> FileObject fileobject= ...
>
> if(!new File(fileobject.toURI().exists())
>      throw new FileNotFoundException();
> }
>
> Unfortunately this would include another creation of an URI object but 
> FileObject..getName() only returns a "user friendly" file name which I 
> understand is not necessarily useable for a File-Object creation.
> The fileobject involved in getResource & createResource is actually a 
> com.sun.tools.javac.file.RegularFileObject with a File object as 
> field. So another solution would be to either expose this File object 
> or add an exists() method to RegularFileObject which delegates the 
> call to file Object and cast the FileObject to a RegularFileObject. 
> (too ugly I believe).

You cannot call "new File(...)" or more generally, use any of the File 
API outside of the javac.file package, except to some extent in the 
command line processing classes (Main and JavaCompiler.) This is because 
the compilation units might not be coming directly from files in the 
host file system.   In an IDE using JSR199, for example, the compilation 
units might come from a database or from string buffers, depending on 
the implementation of JavaFileManager in use. The only real solution 
would be to add a new exists() method to FileObject and use that, but 
that too would be problematic because that would be a 
binary-incompatible change. The best we can probably do in the short 
term is to test if the
fileObject is an instance of RegularFileObject or perhaps 
BaseFileObject, and only apply the check in that case. Which is somewhat 
uglier than your "too ugly I believe" but arguably the best we can do, 
for now. :-(

Longer term, I note as background that JavaFileManager was designed as a 
"virtual file system as defined by the needs of javac". Some wise person 
once made comments about "as simple as possible but no simpler".  I 
think JavaFileManager ended up a tad too simple.  Separately, JSR 203 
("Son of NIO") is providing a more general virtual file system API.  I 
think it would be interesting to see an impl of 
JavaFileManager/FileObject/JavaFileObject built on top of the JSR 203 
APIs. This would then allow applications that wanted to use JSR199 to 
drive the compiler using a "non-standard" file system to use the JSR 203 
API to describe that file system instead of the JavaFileManager API.  
Put another way, if JSR203 had been available in time for JDK 6, I don't 
think we would have bothered with JavaFileManager etc.  The point of all 
of which is to say that the way forward here is to (eventually) provide 
an adapter for JSR 203, and to encourage folk to use JSR 203 as a way to 
provide virtual file systems.

>
> Thanks
>    Emil
>
>
>
>
>
>
>
>
>
>
> ------------------------------------------------------------------------
> *Von:* Jonathan Gibbons <Jonathan.Gibbons at Sun.COM>
> *An:* Emil Siemes <digitalemil at yahoo.de>
> *CC:* compiler-dev at openjdk.java.net
> *Gesendet:* Mittwoch, den 7. Januar 2009, 18:07:35 Uhr
> *Betreff:* Re: AW: [Patch] 6502392 Invalid relative names for 
> Filer.createResource and Filer.getResource
>
> Emil,
>
> First, some updates.
> 1) 6791041 has come in, which reports a specific instance of a user 
> encountering the issue you have been looking at. Specifically,
> the submitter is affected by the "quick and dirty" check in isRelativeURI.
> 2) I have filed 6791060 against java.net.URI
>
> It sounds like you are reasonably familiar with the RFCs involved 
> here. If so, can you briefly summarize any relevant differences
> between RFC 2396 (used by java.net.URI) and RFC 3986 (used by JSR 269 
> and Filer.)
>
> -- Jon
>
> On Jan 6, 2009, at 8:21 AM, Emil Siemes wrote:
>
>> Hi Jon,
>>
>> true, the code actually does things which would belong into 
>> java.net.URI. Reason for this is that JSR269 requires compliance with 
>> path-rootless rule of RFC3986 whereas java.net.URI supports the older 
>> and obsolete RFC 2396. Basically I was afraid of relying on the old 
>> implementation but that can be changed of course.
>> Given the limitation of the actual URI implementation it might be 
>> tricky to achieve compliance with the JSR269 requirements without 
>> parsing URI..getPath(). Dot-segments are not allowed as relative path 
>> names in JSR269 but are fine for the URI class. The current 
>> implementation calls URI.normalize().getPath() which actually removes 
>> dot-segments and therefore doesn't throw an exception. 
>>
>> Given your feedback I propose I rewrite my code in a way that it 
>> relies on java.net.URI and does a quick check on the path. May be I 
>> can compare URI.getPath() and URI.normalize().getPath() to find out 
>> if any dot-segments are part of the path. I'll check and let you know.
>>
>> Thanks
>>    Emil
>>
>>
>>  
>>
>>
>> ------------------------------------------------------------------------
>> *Von:* Jonathan Gibbons <Jonathan.Gibbons at Sun.COM 
>> <mailto:Jonathan.Gibbons at Sun.COM>>
>> *An:* Emil Andreas Siemes <digitalemil at yahoo.de 
>> <mailto:digitalemil at yahoo.de>>
>> *CC:* compiler-dev at openjdk.java.net 
>> <mailto:compiler-dev at openjdk.java.net>
>> *Gesendet:* Dienstag, den 6. Januar 2009, 02:11:22 Uhr
>> *Betreff:* Re: [Patch] 6502392 Invalid relative names for 
>> Filer.createResource and Filer.getResource
>>
>> Emil,
>>
>> Thank you for looking at this, and for your patch.
>>
>> I do not understand why 6419701 is not public. I am trying to find 
>> out why it is not.
>> The bug itself is about the use of URI.create, not about the use of 
>> isRelativeURI..
>>
>> I'm not sure that your patch is warranted (or arguably, the correct 
>> fix).  Your patch certainly 
>> contains functionality that on the face of it would better belong in 
>> java.net.URI and not 
>> JavacFileManager. 
>>
>> The way I read the code in JavacFileManager, isRelativeURI is not 
>> intended to be a full and 
>> complete implementation of checking whether a URI is path-rootless. 
>> Instead, it is used in 
>> exactly 3 places, and always in conjunction with URI.create.  
>> JavacFileManager is assuming 
>> the full syntax checking is done by URI.create, and is just using a 
>> simple impl of 
>> isRelativeURI to distinguish relative URIs from absolute URIs created 
>> by URI.create.
>>
>> 6419701 is about removing the use of URI.create -- presumably because 
>> it is mildly icky
>> to create the URI object just to check whether a path string is a 
>> valid URI.  So it seems 
>> to me that there are two ways to take this issue:  
>> 1) Rewrite isRelativeURI to take a String argument and thereby avoid 
>> the use of URI.create
>> 2) Wait for URI to have some better method for checking the validity 
>> of a path without
>> actually constructing the URI object.
>>
>> I guess that I dislike JavacFileManager having code for detailed 
>> parsing of URIs, and so
>> I think the current impl is better, of using URI.create to check the 
>> syntax of a URI, and
>> then apply a quick check as to whether it is absolute or relative is 
>> better.
>>
>> Or am I misreading the situation or perhaps missing some subtlety in 
>> your proposed code?
>>
>> -- Jon
>>
>>
>> Emil Andreas Siemes wrote:
>>> com.sun.tools.javac.file.JavacFileManager checks if a name fulfills 
>>> the requirements of relative names:
>>> "A relative name is a non-null, non-empty sequence of path segments 
>>> separated by |'/'|; |'.'| and |'..'| are invalid path segments. A 
>>> valid relative name must match the "path-rootless" rule of RFC 3986 
>>> <http://www.ietf.org/rfc/rfc3986.txt>, section 3.3."
>>>
>>> This is done via the isRelativeUri() method. Unfortunately this 
>>> method doesn't currently really check for path-rootless compliance. 
>>> A patch with an improved method is attached.
>>> When java.net.Uri eventually supports RFC 3986 the need for 
>>> isRelativeUri() goes away.. I would also like to propose to rename 
>>> isRelativeUri() to isRF3986PathRootless() which would more clearly 
>>> describe what the method does.
>>>
>>> There is a comment in JavacFileManager in a couple of lines:
>>> //FIX ME 6419701
>>> but I don't find bug 6419701 in the bug database. Any thoughts?
>>>
>>> Thanks
>>>     Emil
>>>
>>>
>>>
>>>
>>>  
>>>
>>> ------------------------------------------------------------------------
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20090108/12f40760/attachment.html