Make NoDecode the default setting for AllowEncodedSlashes #469#470
Make NoDecode the default setting for AllowEncodedSlashes #469#470jamesp-epcc wants to merge 1 commit intorucio:masterfrom
Conversation
|
As this looks like a breaking change, can you elaborate a bit about the possible implications, especially in the commit message? |
|
Sure. In most cases that I'm aware of, this change should be safe. The main reason we want it is so that we can use a simpler decoding method on the server to split a scope and name within a URL and get rid of some complex code. Without this option being enabled, we can't do that, because some experiments use names that start with a forward slash, and this gets merged with the separator slash by the web server. The main implication of enabling this setting is that any encoded slashes (i.e. I think this explanation is a bit too long to put in the commit message but I can add something there if people think that would be useful. |
|
I dont think you need to worry about the length of a commit message.. Mind you, I am not talking about the first line (summary). I just grabbed the first commit I saw on the linux kernel to make my point: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=v6.18.6&id=81531bdea972c3cd8f147f61ca6027fa24e08fc3 |
jamesp-epcc
force-pushed
the
feature-469-allow-encoded-slashes-nodecode
branch
from
e2cdf85 to
c671258
Compare
|
I've added it to the commit message. |
|
I was also looking for the removal of |
This removes the current RUCIO_HTTPD_ENCODED_SLASHES_NO_DECODE and RUCIO_HTTPD_ENCODED_SLASHES settings so that the Apache option AllowEncodedSlashes is always set to NoDecode. In most cases that I'm aware of, this change should be safe. The main reason we want it is so that we can use a simpler decoding method on the server to split a scope and name within a URL and get rid of some complex code. Without this option being enabled, we can't do that, because some experiments use names that start with a forward slash, and this gets merged with the separator slash by the web server. The main implication of enabling this setting is that any encoded slashes (i.e. %2F) in URLs will not be merged with adjacent slashes. I'm not aware of any cases where Rucio relies on this happening and this has been a supported option for some time, just not the default.
jamesp-epcc
force-pushed
the
feature-469-allow-encoded-slashes-nodecode
branch
from
c671258 to
dc604e6
Compare
|
I've mentioned explicitly that those options will be removed. |
bziemons
left a comment
bziemons
left a comment
There was a problem hiding this comment.
I haven't found any other mentions of these constants and I assume the change has been discussed in a Rucio Dev meeting, since it does affects the community deployments. If nobody sees any issue with this change, I am fine with it.
|
I think the change is fine, but I believe something like this should probably be merged for a major release. There is a small chance that it has side effects, which would be not ideal if we introduce this in a minor release. |
|
It's fine if it waits for the major release. My goal was to get the scope name regexp changes (which is dependent on this) into the next major release. |
3 participants
Having this as the default setting will allow us to simplify the parsing of scopes and names on the server and close rucio/rucio#7519 .