Thanks for the discussion Olle, the fix has been merged.
···
On 3 February 2015 at 14:45, Olle Markljung <markljung@anonymised.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
–
Jody Garnett
On Tue, Feb 3, 2015 at 2:26 AM, Jody Garnett <jody.garnett@anonymised.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 <markljung@anonymised.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.garnett@anonymised.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 <markljung@anonymised.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.garnett@anonymised.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 <markljung@anonymised.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 <markljung@anonymised.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.aime@anonymised.com> wrote:
On Mon, Jan 19, 2015 at 9:29 AM, Olle Markljung <markljung@anonymised.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.
