Thanks for the discussion Olle, the fix has been merged.
--
Jody Garnett
On 3 February 2015 at 14:45, Olle Markljung <marklj...@gmail.com> wrote:
> Yes it will.
> But the problem here is that we do not know the intention of the user.
> Existing code has a different handling of file:// urls on windows than on
> other platforms.
> On Windows file://images/poi.png will be converted into \\images\poi.png.
> An absolute path to the host images.
> On other platforms the same url will be converted into images/poi.png. A
> relative path.
>
> So, by running existing code and se if we find a file at that location the
> backwards compatibility will not be broken.
> And by assuming relative path if no file exist you get the same result as
> for other platforms.
>
> Since the Paths.VALID does not allow for backslashes "backslash (sorry
> windows users we are following linux conventions here)" conversion code is
> needed when converting URLs to resource in GeoServerDataDirectory.java.
> If I add that the build is successful with mvn clean install.
> I'll try to formulate a suiting JIRA and send a PR for the change.
>
> /Olle
>
> On Tue, Feb 3, 2015 at 2:26 AM, Jody Garnett <jody.garn...@gmail.com>
> wrote:
>
>> Since (in GeoServer) we try to discourage the use of absolute paths I
>> guess I am fine with the way things stand.
>>
>> If we were just focused on manipulating the file path in a safe manner
>> the Java Paths API would help (since we actually want to check a file on
>> disk I expect the Files API is fine).
>>
>> Correct me if I am wrong, and absolute path on windows will end up using
>> the C:\ notation or the \\host\share\ notation - in both cases these are
>> pretty specific and not confusable as a relative path?
>>
>> --
>> Jody Garnett
>>
>> On 2 February 2015 at 14:52, Olle Markljung <marklj...@gmail.com> wrote:
>>
>>> Yes, it checks if the file at the absolute path exists and if not
>>> assumes the path is relative.
>>> You could swap the order making the relative path default.
>>>
>>> Fail? The tests use paths to files that does not exist.
>>> The result will be that the path to the file will be relative. And
>>> that's the same result that you get on other platforms.
>>>
>>> But yes, if your intention is to use an absolute path to something that
>>> does not exist yet it will fail.
>>> So, by swapping the order you would get the absoulte file path as the
>>> default on windows as before for files that do not exist yet.
>>> But if there is a file there for the relative case that will be used
>>> instead. However that seems a bit unlikely.
>>> Should I update the PR to do that?
>>>
>>> Still not sure how the Java Path API would help here.
>>>
>>> /Olle
>>>
>>>
>>> On Sun, Feb 1, 2015 at 11:26 PM, Jody Garnett <jody.garn...@gmail.com>
>>> wrote:
>>>
>>>> One of the lines in your pull request uses the test if (!f.exists())
>>>>
>>>> This only works if the DataUtilities.urlToFile method is referring to a
>>>> file that has been created yet. If we try the same logic on a file that
>>>> does not exist yet it will fail...
>>>>
>>>> --
>>>> Jody Garnett
>>>>
>>>> On 1 February 2015 at 06:36, Olle Markljung <marklj...@gmail.com>
>>>> wrote:
>>>>
>>>>> Sorry for the delay.
>>>>>
>>>>> Ticket: http://jira.codehaus.org/browse/GEOT-4990
>>>>> PR: https://github.com/geotools/geotools/pull/717
>>>>>
>>>>> This builds clean using "mvn clean install" on my machine (building
>>>>> geotools).
>>>>> Should I communicate this on the geotools list as well?
>>>>>
>>>>> Not sure that I understand what you mean Jody.
>>>>> If I have gotten this right the problem is to know if the user provide
>>>>> an absolute or relative path.
>>>>> How would the path API help in knowing the intentions of the user?
>>>>>
>>>>> /Olle
>>>>>
>>>>> On Wed, Jan 21, 2015 at 12:33 AM, Jody Garnett <jody.garn...@gmail.com
>>>>> > wrote:
>>>>>
>>>>>> Sounds like a good approach - we may also be able to use Java 7 Path
>>>>>> API.
>>>>>>
>>>>>> I should also point out that we may be using this to figure out a the
>>>>>> location of a *new* file (like one that does not exist yet). Perhaps the
>>>>>> Java 7 path api can help.
>>>>>> --
>>>>>> Jody
>>>>>>
>>>>>> --
>>>>>> Jody Garnett
>>>>>>
>>>>>> On 20 January 2015 at 15:28, Olle Markljung <marklj...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Ok,
>>>>>>> Perhaps it is not easy to say in what ways it all should work.
>>>>>>> Someone ought to be depending on the code doing this specific thing on
>>>>>>> Windows since the code exists.
>>>>>>> So, I got a proposition.
>>>>>>> What if we in DataUtilities.urlToFile do the same as
>>>>>>> DefaultResourceLocator.locateResource already does.
>>>>>>> That is to check if the file exists. If it doesn't we remove the
>>>>>>> extra slashes and makes the URI relative.
>>>>>>>
>>>>>>> I extended the tests and added such code to the Windows specific
>>>>>>> case and it works as expected.
>>>>>>> Should I create a ticket and send a PR with these changes for your
>>>>>>> review?
>>>>>>>
>>>>>>> /Olle
>>>>>>>
>>>>>>> On Mon, Jan 19, 2015 at 11:42 PM, Olle Markljung <
>>>>>>> marklj...@gmail.com> wrote:
>>>>>>>
>>>>>>>> Ok.
>>>>>>>> Looking at the history I can't find anything that changed. Version
>>>>>>>> of Java did change to 7.
>>>>>>>>
>>>>>>>> These urls also fail in GeoTools.
>>>>>>>> It's not the usage of urlToFile in the test that's the problem but
>>>>>>>> the usage of urlToFile in the DefaultResourceLocator.locateResource.
>>>>>>>> The file:// is converted to // by urlToFile and this means it's not
>>>>>>>> relative. You'll get the same behavior if you use file:/ instead.
>>>>>>>> Therefore locateResource will leave the URI as is and not make it
>>>>>>>> relative to the styles folder.
>>>>>>>>
>>>>>>>> So, on Windows the urls will be interpreted as absolute but on
>>>>>>>> other platforms it will be relative. Atleast the file:// case.
>>>>>>>> Should file://host/share/dest.png be supported on Windows but not
>>>>>>>> elsewhere?
>>>>>>>> Would \\host\share\dest.png work an be a acceptable replacement?
>>>>>>>> Is it intentional that file:/ will be an absolute path and file://
>>>>>>>> not?
>>>>>>>>
>>>>>>>> Something that does work is removing the forward slashes. So,
>>>>>>>> file:dest.png will give you the correct behavior. As
>>>>>>>> https://jira.codehaus.org/browse/GEOT-4311 says.
>>>>>>>> I'm merely trying to understand the requirements so go easy on me :)
>>>>>>>>
>>>>>>>> /Olle
>>>>>>>>
>>>>>>>> On Mon, Jan 19, 2015 at 9:40 AM, Andrea Aime <
>>>>>>>> andrea.a...@geo-solutions.it> wrote:
>>>>>>>>
>>>>>>>>> On Mon, Jan 19, 2015 at 9:29 AM, Olle Markljung <
>>>>>>>>> marklj...@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>> Yes, I see now that I was a bit unclear with my intentions of the
>>>>>>>>>> last paragraph.
>>>>>>>>>>
>>>>>>>>>> However, I believe that the usage of the file protocol makes it
>>>>>>>>>> unclear as when the file path will be interpreted as relative over
>>>>>>>>>> absolute.
>>>>>>>>>>
>>>>>>>>>> I can get back to you with some exemples and maybe we can
>>>>>>>>>> document the requirements and expected behavior.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The test is checking for behavior that was working up to 2.5.x.
>>>>>>>>> Both worked:
>>>>>>>>> file://dest.png
>>>>>>>>> file.//./dest.png
>>>>>>>>>
>>>>>>>>> Cheers
>>>>>>>>> Andrea
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> ==
>>>>>>>>> GeoServer Professional Services from the experts! Visit
>>>>>>>>> http://goo.gl/NWWaa2 for more information.
>>>>>>>>> ==
>>>>>>>>>
>>>>>>>>> Ing. Andrea Aime
>>>>>>>>> @geowolf
>>>>>>>>> Technical Lead
>>>>>>>>>
>>>>>>>>> GeoSolutions S.A.S.
>>>>>>>>> Via Poggio alle Viti 1187
>>>>>>>>> 55054 Massarosa (LU)
>>>>>>>>> Italy
>>>>>>>>> phone: +39 0584 962313
>>>>>>>>> fax: +39 0584 1660272
>>>>>>>>> mob: +39 339 8844549
>>>>>>>>>
>>>>>>>>> http://www.geo-solutions.it
>>>>>>>>> http://twitter.com/geosolutions_it
>>>>>>>>>
>>>>>>>>> *AVVERTENZE AI SENSI DEL D.Lgs. 196/2003*
>>>>>>>>>
>>>>>>>>> Le informazioni contenute in questo messaggio di posta elettronica
>>>>>>>>> e/o nel/i file/s allegato/i sono da considerarsi strettamente
>>>>>>>>> riservate. Il
>>>>>>>>> loro utilizzo è consentito esclusivamente al destinatario del
>>>>>>>>> messaggio,
>>>>>>>>> per le finalità indicate nel messaggio stesso. Qualora riceviate
>>>>>>>>> questo
>>>>>>>>> messaggio senza esserne il destinatario, Vi preghiamo cortesemente di
>>>>>>>>> darcene notizia via e-mail e di procedere alla distruzione del
>>>>>>>>> messaggio
>>>>>>>>> stesso, cancellandolo dal Vostro sistema. Conservare il messaggio
>>>>>>>>> stesso,
>>>>>>>>> divulgarlo anche in parte, distribuirlo ad altri soggetti, copiarlo,
>>>>>>>>> od
>>>>>>>>> utilizzarlo per finalità diverse, costituisce comportamento contrario
>>>>>>>>> ai
>>>>>>>>> principi dettati dal D.Lgs. 196/2003.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The information in this message and/or attachments, is intended
>>>>>>>>> solely for the attention and use of the named addressee(s) and may be
>>>>>>>>> confidential or proprietary in nature or covered by the provisions of
>>>>>>>>> privacy act (Legislative Decree June, 30 2003, no.196 - Italy's New
>>>>>>>>> Data
>>>>>>>>> Protection Code).Any use not in accord with its purpose, any
>>>>>>>>> disclosure,
>>>>>>>>> reproduction, copying, distribution, or either dissemination, either
>>>>>>>>> whole
>>>>>>>>> or partial, is strictly forbidden except previous formal approval of
>>>>>>>>> the
>>>>>>>>> named addressee(s). If you are not the intended recipient, please
>>>>>>>>> contact
>>>>>>>>> immediately the sender by telephone, fax or e-mail and delete the
>>>>>>>>> information in this message that has been received in error. The
>>>>>>>>> sender
>>>>>>>>> does not give any warranty or accept liability as the content,
>>>>>>>>> accuracy or
>>>>>>>>> completeness of sent messages and accepts no responsibility for
>>>>>>>>> changes
>>>>>>>>> made after they were sent or for other risks which arise as a result
>>>>>>>>> of
>>>>>>>>> e-mail transmission, viruses, etc.
>>>>>>>>>
>>>>>>>>> -------------------------------------------------------
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
