You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by dudaerich <gi...@git.apache.org> on 2016/10/14 10:09:46 UTC
[GitHub] activemq-artemis pull request #845: ARTEMIS-801 Decode URL paths
GitHub user dudaerich opened a pull request:
https://github.com/apache/activemq-artemis/pull/845
ARTEMIS-801 Decode URL paths
If the path to file contains some special characters, they are encoded
in URL form using %<hex> syntax. We should decode such path when it
is used as path to file on local filesystem.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dudaerich/activemq-artemis ARTEMIS-801
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/845.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #845
----
commit 29d5cd2b58aab9b09eccc66ef0eb16e185e26dda
Author: Erich Duda <ed...@redhat.com>
Date: 2016-10-14T07:03:22Z
ARTEMIS-801 Decode URL paths
If the path to file contains some special characters, they are encoded
in URL form using %<hex> syntax. We should decode such path when it
is used as path to file on local filesystem.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #845: ARTEMIS-801 Decode URL paths
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/845
@dudaerich I have committed a fix with this PR. Please take a look, it is a more complete fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #845: ARTEMIS-801 Decode URL paths
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/845
@dudaerich can you close your PR?
You hit an issue, but you are fixing the test, not the issue... I mean.. this will hide the issue not fix it.
I will provide a proper fix using everything you found here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #845: ARTEMIS-801 Decode URL paths
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/845#discussion_r83682900
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java ---
@@ -92,7 +95,17 @@ private ReloadRegistry getRegistry(URL uri) {
private final List<ReloadCallback> callbacks = new LinkedList<>();
ReloadRegistry(URL uri) {
- this.file = new File(uri.getPath());
+ String filePath = null;
+ try {
+ filePath = URLDecoder.decode(uri.getPath(), StandardCharsets.UTF_8.name());
--- End diff --
hmmm.. the issue is on the reload manager only?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #845: ARTEMIS-801 Decode URL paths
Posted by dudaerich <gi...@git.apache.org>.
Github user dudaerich commented on the issue:
https://github.com/apache/activemq-artemis/pull/845
Hi @clebertsuconic
my intention here was to fix tests which fail in our CI. The path to project directory contains '&' characters. Example of path:
```
.../jdk/java18_default/label/RHEL6&&x86_64/...
```
I didn't realize that this could cause problems at more places in the code. Using URI to open a file is much better solution than decoding the URL :). Thanks for the proper fix, it looks good. I executed the tests with your fix and they passed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #845: ARTEMIS-801 Decode URL paths
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/845
do you have an example of such path?
I suggest you also look at artemis-distribution/src/test/scripts/validate-spaces.sh
I always run validate-spaces.sh.. I even have a little script on my dev environemnt to always run it. so maybe we could expand validate-spaces to also validate such thing?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #845: ARTEMIS-801 Decode URL paths
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/845#discussion_r83683002
--- Diff: artemis-server/src/test/java/org/apache/activemq/artemis/core/security/jaas/GuestLoginModuleTest.java ---
@@ -23,22 +23,33 @@
import javax.security.auth.login.LoginContext;
import javax.security.auth.login.LoginException;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
import java.net.URL;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
import org.apache.activemq.artemis.spi.core.security.jaas.RolePrincipal;
import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal;
+import org.jboss.logging.Logger;
import org.junit.Assert;
import org.junit.Test;
public class GuestLoginModuleTest extends Assert {
+ private static final Logger logger = Logger.getLogger(GuestLoginModuleTest.class);
+
static {
String path = System.getProperty("java.security.auth.login.config");
if (path == null) {
URL resource = GuestLoginModuleTest.class.getClassLoader().getResource("login.config");
if (resource != null) {
- path = resource.getFile();
- System.setProperty("java.security.auth.login.config", path);
+ try {
+ path = URLDecoder.decode(resource.getFile(), StandardCharsets.UTF_8.name());
--- End diff --
I'm just confused why you changed this, if this is just a test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #845: ARTEMIS-801 Decode URL paths
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/845
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---