Hello Jody,
I feel you are not understanding me, it seems you are not really
responding to any my points.
I disagree with your analysis. There is no need to break half to code
base. In my opinion there is nothing wrong with the original idea and
that is already how nearly all of the code
Do I understand correctly,
You have made a long email about asking that an absolute path be treated as
relative to data directory.
The problem we are faced with is that the code base has two inconsistent
uses of absolute path.
No matter what decision made we are going to break half the code
Hello Jody,
I have been looking into this, but I do not think that throwing an
exception with absolute paths is a viable option. It is too problematic
for backwards compatibility. For one it seems to completely break the
"file chooser" in web-core, and it causes problems all around.
The
I think we make the change, so resource store fails with absolute path, and
learn what other areas of the code assume resource store can access any
location on disk.
Then we will know how widespread this problems is.
I am happy to split up the work, and we can add to the documentation a very
Hey Jody,
How do we proceed practically?
I am happy to make a PR to fix resource store and make the agreed upon
core changes.
Will you change the docs or would you like this to be part of my work?
Kind Regards
Niels
On 13/07/2023 16:19, Jody Garnett wrote:
I already tried to catch a lot
Gabe? What do you think?
--
Jody Garnett
On Jul 11, 2023 at 1:27:26 PM, Jody Garnett wrote:
> Thanks:
>
> Reading the docs, it is *okay* but not *great*.
>
> I think we we made it so the ResourceStore implementations fail when
> provided an absolute path it would be best; and remove the test
I already tried to catch a lot of those when adjusting for the windows
absolute paths.
We could add some logic and provide a warning if any absolute paths get
through? And the call Resources.fromURL(base, url) …
Still we may be getting into a technical detail here. What needs to be
changed or
In theory I agree but there is always a risk to changing lenient
behavior to throwing exceptions, if there is code or production configs
that have relied on paths starting with '/' just passing as relative...
On 11/07/2023 22:27, Jody Garnett wrote:
Thanks:
Reading the docs, it is /okay/ but
Thanks:
Reading the docs, it is *okay* but not *great*.
I think we we made it so the ResourceStore implementations fail when
provided an absolute path it would be best; and remove the test case that
is causing problem.
That way only Resources.fromURL(base, url) would support "absolute" paths
Hello Jody,
My comment about deprecating the file() was in response to what Gabriel
said about that.
Resource: urls are indeed for instance supported by SLD, if you want to
link to a resource inside your data directory / resource store. But I
think they can be used about anywhere where
Niels,
I do not understand the comment about deprecating the file() method? I
agree with everything you said about how it is used and useful.
I am not sure when resource URLs were added, I thought they were only added
to pass information over to geotools side of things for SLD validation and
Hello Gabriel and Jody,
I agree with most of what Gabriel says, it is mainly the same point as
mine except that I think it is unrealistic to deprecate the file()
method. First of all we would need to to move the resource API to
geotools because SLD files are stored in the resource store, but
Hey folks, I was tired and not in position to act clearly when these gaps
were noticed last year.
We can tighten up the api definition, at least with the changes made we can
now notice when an absolute path was used.
I never quite managed to communicate the separation between using a file
URL
If "the codebase had drifted away from the intended use over time" I think
it's even more important to stick to the contract and not the other way
around.
As far as I can see, there are two abstractions, ResourceStore and
Resource, the former clearly says
"This abstraction assumes a unix like file
Hello Jody,
Yes, I admit it's my own fault for missing this discussion at the time.
I think it would be a shame to let the codebase further drift away from
the intended use of the resource store. We have done the work to make
the entirety of geoserver generic with respect to the
Hello Niels,
The PRs for this activity contain extensive discussion.
The fundamental issue was the handling of absolute paths which was done
differently by different sections of code. Specifically we found that the
REST API endpoint was allowing paths *//data* and */data *to both reference
Hello Jody and others,
I am having trouble understanding the changes that were made about 6
months ago to the ResourceStore's expected behaviour.
In particular, in the class
'org.geoserver.platform.resource.ResourceTheoryTest', the unit test
'theoryRootSlashIsIgnored' was replaced by
17 matches
Mail list logo
