You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by mi...@apache.org on 2019/07/09 13:55:25 UTC
[tomcat] branch mark-forwarded-request/8.5.x created (now 644af22)
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a change to branch mark-forwarded-request/8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
at 644af22 Better attribute name
This branch includes the following new commits:
new 2eeb813 Mark request as forwarded in RemoteIpValve/RemoteIpFilter
new 644af22 Better attribute name
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch mark-forwarded-request/8.5.x created (now
644af22)
Posted by Mark Thomas <ma...@apache.org>.
On 09/07/2019 21:09, Rémy Maucherat wrote:
> On Tue, Jul 9, 2019 at 3:55 PM <michaelo@apache.org
> <ma...@apache.org>> wrote:
>
> This is an automated email from the ASF dual-hosted git repository.
>
> michaelo pushed a change to branch mark-forwarded-request/8.5.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git.
>
>
> at 644af22 Better attribute name
>
> This branch includes the following new commits:
>
> new 2eeb813 Mark request as forwarded in
> RemoteIpValve/RemoteIpFilter
> new 644af22 Better attribute name
>
> The 2 revisions listed above as "new" are entirely new to this
> repository and will be described in separate emails. The revisions
> listed as "add" were already present in the repository and have only
> been added to this reference.
>
>
> Personally, I don't like the idea of using random branches in the main
> repository vs using forks.
>
> Should it be explicitly required to use forks with PRs for reviews ?
I'm undecided.
I will note that we can delete these additional branches once they are
no longer required.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: [tomcat] branch mark-forwarded-request/8.5.x created (now 644af22)
Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Jul 9, 2019 at 3:55 PM <mi...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> michaelo pushed a change to branch mark-forwarded-request/8.5.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git.
>
>
> at 644af22 Better attribute name
>
> This branch includes the following new commits:
>
> new 2eeb813 Mark request as forwarded in RemoteIpValve/RemoteIpFilter
> new 644af22 Better attribute name
>
> The 2 revisions listed above as "new" are entirely new to this
> repository and will be described in separate emails. The revisions
> listed as "add" were already present in the repository and have only
> been added to this reference.
>
Personally, I don't like the idea of using random branches in the main
repository vs using forks.
Should it be explicitly required to use forks with PRs for reviews ?
Rémy
[tomcat] 01/02: Mark request as forwarded in
RemoteIpValve/RemoteIpFilter
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch mark-forwarded-request/8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2eeb813dad2bcda58959b6eeea468d41f998f57b
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Tue Jul 9 14:59:09 2019 +0200
Mark request as forwarded in RemoteIpValve/RemoteIpFilter
---
java/org/apache/catalina/Globals.java | 10 ++++++++
.../apache/catalina/filters/RemoteIpFilter.java | 4 +++
java/org/apache/catalina/valves/RemoteIpValve.java | 4 +++
java/org/apache/coyote/Constants.java | 8 ++++++
.../catalina/filters/TestRemoteIpFilter.java | 23 +++++++++++++++++
.../apache/catalina/valves/TestRemoteIpValve.java | 30 ++++++++++++++++++++++
webapps/docs/changelog.xml | 7 +++++
7 files changed, 86 insertions(+)
diff --git a/java/org/apache/catalina/Globals.java b/java/org/apache/catalina/Globals.java
index 8801724..edf91a0 100644
--- a/java/org/apache/catalina/Globals.java
+++ b/java/org/apache/catalina/Globals.java
@@ -199,6 +199,16 @@ public final class Globals {
org.apache.coyote.Constants.REMOTE_ADDR_ATTRIBUTE;
+ /**
+ * The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
+ * be set by other similar components) that identifies this request has been
+ * forwarded via one or more proxies. The value should be {@code java.lang.Boolean}.
+ * Absence shall be treated as {@code false}.
+ */
+ public static final String FORWARDED_REQUEST_ATTRIBUTE =
+ org.apache.coyote.Constants.FORWARDED_REQUEST_ATTRIBUTE;
+
+
public static final String ASYNC_SUPPORTED_ATTR =
"org.apache.catalina.ASYNC_SUPPORTED";
diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java
index 20b4abd..f416f60 100644
--- a/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ b/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -85,6 +85,8 @@ import org.apache.juli.logging.LogFactory;
* <code>protocolHeaderHttpsValue</code> configuration parameter (default <code>https</code>) then <code>request.isSecure = true</code>,
* <code>request.scheme = https</code> and <code>request.serverPort = 443</code>. Note that 443 can be overwritten with the
* <code>$httpsServerPort</code> configuration parameter.</li>
+ * <li>Mark the request with the attribute {@link Globals#FORWARDED_REQUEST_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
+ * that this request has been forwarded by one or more proxies.</li>
* </ul>
* <table border="1">
* <caption>Configuration parameters</caption>
@@ -860,6 +862,8 @@ public class RemoteIpFilter implements Filter {
}
}
+ request.setAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE, Boolean.TRUE);
+
if (log.isDebugEnabled()) {
log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr '" + request.getRemoteAddr()
+ "', originalRemoteHost='" + request.getRemoteHost() + "', originalSecure='" + request.isSecure()
diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index 145b095..9e78c0f 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -64,6 +64,8 @@ import org.apache.tomcat.util.http.MimeHeaders;
* <code>protocolHeaderHttpsValue</code> configuration parameter (default <code>https</code>) then <code>request.isSecure = true</code>,
* <code>request.scheme = https</code> and <code>request.serverPort = 443</code>. Note that 443 can be overwritten with the
* <code>$httpsServerPort</code> configuration parameter.</li>
+ * <li>Mark the request with the attribute {@link Globals#FORWARDED_REQUEST_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
+ * that this request has been forwarded by one or more proxies.</li>
* </ul>
* <table border="1">
* <caption>Configuration parameters</caption>
@@ -651,6 +653,8 @@ public class RemoteIpValve extends ValveBase {
}
}
+ request.setAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE, Boolean.TRUE);
+
if (log.isDebugEnabled()) {
log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr '" + originalRemoteAddr
+ "', originalRemoteHost='" + originalRemoteHost + "', originalSecure='" + originalSecure + "', originalScheme='"
diff --git a/java/org/apache/coyote/Constants.java b/java/org/apache/coyote/Constants.java
index 9de194d..58fa1e5 100644
--- a/java/org/apache/coyote/Constants.java
+++ b/java/org/apache/coyote/Constants.java
@@ -111,4 +111,12 @@ public final class Constants {
* the X-Forwarded-For HTTP header.
*/
public static final String REMOTE_ADDR_ATTRIBUTE = "org.apache.tomcat.remoteAddr";
+
+ /**
+ * The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
+ * be set by other similar components) that identifies this request has been
+ * forwarded via one or more proxies. The value should be {@code java.lang.Boolean}.
+ * Absence shall be treated as {@code false}.
+ */
+ public static final String FORWARDED_REQUEST_ATTRIBUTE = "org.apache.tomcat.forwardedRequest";
}
diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
index acfc881..e8323e9 100644
--- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -42,6 +42,7 @@ import org.junit.Test;
import org.apache.catalina.AccessLog;
import org.apache.catalina.Context;
+import org.apache.catalina.Globals;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.connector.Request;
@@ -625,6 +626,28 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
actualRequest.getAttribute(AccessLog.REMOTE_HOST_ATTRIBUTE));
}
+ @Test
+ public void testRequestForwarded() throws Exception {
+ // PREPARE
+ FilterDef filterDef = new FilterDef();
+ filterDef.addInitParameter("protocolHeader", "x-forwarded-proto");
+ filterDef.addInitParameter("remoteIpHeader", "x-my-forwarded-for");
+ filterDef.addInitParameter("httpServerPort", "8080");
+
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.setRemoteAddr("192.168.0.10");
+ request.setHeader("x-my-forwarded-for", "140.211.11.130");
+ request.setHeader("x-forwarded-proto", "http");
+
+ // TEST
+ HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request).getRequest();
+
+ // VERIFY
+ Assert.assertEquals("org.apache.tomcat.forwardedRequest",
+ Boolean.TRUE,
+ actualRequest.getAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE));
+ }
+
/*
* Test {@link RemoteIpFilter} in Tomcat standalone server
*/
diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index 1778b5a..21d79d3 100644
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -27,6 +27,7 @@ import org.junit.Assert;
import org.junit.Test;
import org.apache.catalina.AccessLog;
+import org.apache.catalina.Globals;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
@@ -980,6 +981,35 @@ public class TestRemoteIpValve {
request.getAttribute(AccessLog.REMOTE_HOST_ATTRIBUTE));
}
+ @Test
+ public void testRequestForwarded() throws Exception {
+
+ // PREPARE
+ RemoteIpValve remoteIpValve = new RemoteIpValve();
+ remoteIpValve.setRemoteIpHeader("x-forwarded-for");
+ remoteIpValve.setProtocolHeader("x-forwarded-proto");
+ RemoteAddrAndHostTrackerValve remoteAddrAndHostTrackerValve = new RemoteAddrAndHostTrackerValve();
+ remoteIpValve.setNext(remoteAddrAndHostTrackerValve);
+
+ Request request = new MockRequest();
+ request.setCoyoteRequest(new org.apache.coyote.Request());
+ // client ip
+ request.setRemoteAddr("192.168.0.10");
+ request.setRemoteHost("192.168.0.10");
+ request.getCoyoteRequest().getMimeHeaders().addValue("x-forwarded-for").setString("140.211.11.130");
+ // protocol
+ request.setServerPort(8080);
+ request.getCoyoteRequest().scheme().setString("http");
+
+ // TEST
+ remoteIpValve.invoke(request, null);
+
+ // VERIFY
+ Assert.assertEquals("org.apache.tomcat.forwardedRequest",
+ Boolean.TRUE,
+ request.getAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE));
+ }
+
private void assertArrayEquals(String[] expected, String[] actual) {
if (expected == null) {
Assert.assertNull(actual);
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d9d75bc..cb2e02d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -45,6 +45,13 @@
issues do not "pop up" wrt. others).
-->
<section name="Tomcat 8.5.44 (markt)" rtext="in development">
+ <subsection name="Catalina">
+ <changelog>
+ <add>
+ <bug>XXX</bug>: Mark request as forwarded in RemoteIpValve/RemoteIpFilter (michaelo)
+ </add>
+ </changelog>
+ </subsection>
<subsection name="Other">
<changelog>
<update>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Better attribute name
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch mark-forwarded-request/8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 644af221e5cb8e6a17327c9a4319a69a2d1dc305
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Tue Jul 9 15:53:36 2019 +0200
Better attribute name
---
java/org/apache/catalina/Globals.java | 11 +++++------
java/org/apache/catalina/filters/RemoteIpFilter.java | 2 +-
java/org/apache/catalina/valves/RemoteIpValve.java | 4 ++--
java/org/apache/coyote/Constants.java | 9 ++++-----
test/org/apache/catalina/filters/TestRemoteIpFilter.java | 4 ++--
test/org/apache/catalina/valves/TestRemoteIpValve.java | 4 ++--
6 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/java/org/apache/catalina/Globals.java b/java/org/apache/catalina/Globals.java
index edf91a0..7ce8209 100644
--- a/java/org/apache/catalina/Globals.java
+++ b/java/org/apache/catalina/Globals.java
@@ -200,13 +200,12 @@ public final class Globals {
/**
- * The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
- * be set by other similar components) that identifies this request has been
- * forwarded via one or more proxies. The value should be {@code java.lang.Boolean}.
- * Absence shall be treated as {@code false}.
+ * The request attribute that is set to the value of {@code Boolean.TRUE}
+ * by the RemoteIpFilter, RemoteIpValve (and other similar components) that identifies
+ * a request which been forwarded via one or more proxies.
*/
- public static final String FORWARDED_REQUEST_ATTRIBUTE =
- org.apache.coyote.Constants.FORWARDED_REQUEST_ATTRIBUTE;
+ public static final String REQUEST_FORWARDED_ATTRIBUTE =
+ org.apache.coyote.Constants.REQUEST_FORWARDED_ATTRIBUTE;
public static final String ASYNC_SUPPORTED_ATTR =
diff --git a/java/org/apache/catalina/filters/RemoteIpFilter.java b/java/org/apache/catalina/filters/RemoteIpFilter.java
index f416f60..423b561 100644
--- a/java/org/apache/catalina/filters/RemoteIpFilter.java
+++ b/java/org/apache/catalina/filters/RemoteIpFilter.java
@@ -85,7 +85,7 @@ import org.apache.juli.logging.LogFactory;
* <code>protocolHeaderHttpsValue</code> configuration parameter (default <code>https</code>) then <code>request.isSecure = true</code>,
* <code>request.scheme = https</code> and <code>request.serverPort = 443</code>. Note that 443 can be overwritten with the
* <code>$httpsServerPort</code> configuration parameter.</li>
- * <li>Mark the request with the attribute {@link Globals#FORWARDED_REQUEST_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
+ * <li>Mark the request with the attribute {@link Globals#REQUEST_FORWARDED_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
* that this request has been forwarded by one or more proxies.</li>
* </ul>
* <table border="1">
diff --git a/java/org/apache/catalina/valves/RemoteIpValve.java b/java/org/apache/catalina/valves/RemoteIpValve.java
index 9e78c0f..cd08cc7 100644
--- a/java/org/apache/catalina/valves/RemoteIpValve.java
+++ b/java/org/apache/catalina/valves/RemoteIpValve.java
@@ -64,7 +64,7 @@ import org.apache.tomcat.util.http.MimeHeaders;
* <code>protocolHeaderHttpsValue</code> configuration parameter (default <code>https</code>) then <code>request.isSecure = true</code>,
* <code>request.scheme = https</code> and <code>request.serverPort = 443</code>. Note that 443 can be overwritten with the
* <code>$httpsServerPort</code> configuration parameter.</li>
- * <li>Mark the request with the attribute {@link Globals#FORWARDED_REQUEST_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
+ * <li>Mark the request with the attribute {@link Globals#REQUEST_FORWARDED_ATTRIBUTE} and value {@code Boolean.TRUE} to indicate
* that this request has been forwarded by one or more proxies.</li>
* </ul>
* <table border="1">
@@ -653,7 +653,7 @@ public class RemoteIpValve extends ValveBase {
}
}
- request.setAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE, Boolean.TRUE);
+ request.setAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE, Boolean.TRUE);
if (log.isDebugEnabled()) {
log.debug("Incoming request " + request.getRequestURI() + " with originalRemoteAddr '" + originalRemoteAddr
diff --git a/java/org/apache/coyote/Constants.java b/java/org/apache/coyote/Constants.java
index 58fa1e5..898068a 100644
--- a/java/org/apache/coyote/Constants.java
+++ b/java/org/apache/coyote/Constants.java
@@ -113,10 +113,9 @@ public final class Constants {
public static final String REMOTE_ADDR_ATTRIBUTE = "org.apache.tomcat.remoteAddr";
/**
- * The request attribute set by the RemoteIpFilter, RemoteIpValve (and may
- * be set by other similar components) that identifies this request has been
- * forwarded via one or more proxies. The value should be {@code java.lang.Boolean}.
- * Absence shall be treated as {@code false}.
+ * The request attribute that is set to the value of {@code Boolean.TRUE}
+ * by the RemoteIpFilter, RemoteIpValve (and other similar components) that identifies
+ * a request which been forwarded via one or more proxies.
*/
- public static final String FORWARDED_REQUEST_ATTRIBUTE = "org.apache.tomcat.forwardedRequest";
+ public static final String REQUEST_FORWARDED_ATTRIBUTE = "org.apache.tomcat.request.forwarded";
}
diff --git a/test/org/apache/catalina/filters/TestRemoteIpFilter.java b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
index e8323e9..3fb6580 100644
--- a/test/org/apache/catalina/filters/TestRemoteIpFilter.java
+++ b/test/org/apache/catalina/filters/TestRemoteIpFilter.java
@@ -643,9 +643,9 @@ public class TestRemoteIpFilter extends TomcatBaseTest {
HttpServletRequest actualRequest = testRemoteIpFilter(filterDef, request).getRequest();
// VERIFY
- Assert.assertEquals("org.apache.tomcat.forwardedRequest",
+ Assert.assertEquals("org.apache.tomcat.request.forwarded",
Boolean.TRUE,
- actualRequest.getAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE));
+ actualRequest.getAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE));
}
/*
diff --git a/test/org/apache/catalina/valves/TestRemoteIpValve.java b/test/org/apache/catalina/valves/TestRemoteIpValve.java
index 21d79d3..cb26382 100644
--- a/test/org/apache/catalina/valves/TestRemoteIpValve.java
+++ b/test/org/apache/catalina/valves/TestRemoteIpValve.java
@@ -1005,9 +1005,9 @@ public class TestRemoteIpValve {
remoteIpValve.invoke(request, null);
// VERIFY
- Assert.assertEquals("org.apache.tomcat.forwardedRequest",
+ Assert.assertEquals("org.apache.tomcat.request.forwarded",
Boolean.TRUE,
- request.getAttribute(Globals.FORWARDED_REQUEST_ATTRIBUTE));
+ request.getAttribute(Globals.REQUEST_FORWARDED_ATTRIBUTE));
}
private void assertArrayEquals(String[] expected, String[] actual) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org