You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2022/03/23 20:10:33 UTC

[tomcat] branch 10.0.x updated (5563753 -> e186391)

This is an automated email from the ASF dual-hosted git repository.

remm pushed a change to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 5563753  65959: Serialize Function as String[] rather Class[]
     new a82ddf0  PR #487: Improve logging of unknown settings frames
     new e186391  Remove check disabling logging

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.


Summary of changes:
 java/org/apache/coyote/http2/ConnectionSettingsBase.java | 2 --
 java/org/apache/coyote/http2/Http2Parser.java            | 7 ++++++-
 java/org/apache/coyote/http2/Http2UpgradeHandler.java    | 7 ++++++-
 webapps/docs/changelog.xml                               | 4 ++++
 4 files changed, 16 insertions(+), 4 deletions(-)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 02/02: Remove check disabling logging

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit e186391dc9973e922424193baab46e0c520ac359
Author: remm <re...@apache.org>
AuthorDate: Wed Mar 23 21:03:44 2022 +0100

    Remove check disabling logging
---
 java/org/apache/coyote/http2/Http2Parser.java         | 2 +-
 java/org/apache/coyote/http2/Http2UpgradeHandler.java | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 8c67d84..9a4bf0c 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -338,7 +338,7 @@ class Http2Parser {
                 int id = ByteUtil.getTwoBytes(setting, 0);
                 long value = ByteUtil.getFourBytes(setting, 2);
                 Setting key = Setting.valueOf(id);
-                if (log.isDebugEnabled() && key == Setting.UNKNOWN) {
+                if (key == Setting.UNKNOWN) {
                     log.warn(sm.getString("connectionSettings.unknown",
                         connectionId, Integer.toString(id), Long.toString(value)));
                 }
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 75bdac1..03dc48c 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -232,7 +232,7 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                     int id = ByteUtil.getTwoBytes(settings, i * 6);
                     long value = ByteUtil.getFourBytes(settings, (i * 6) + 2);
                     Setting key = Setting.valueOf(id);
-                    if (log.isDebugEnabled() && key == Setting.UNKNOWN) {
+                    if (key == Setting.UNKNOWN) {
                         log.warn(sm.getString("connectionSettings.unknown",
                             connectionId, Integer.toString(id), Long.toString(value)));
                     }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 01/02: PR #487: Improve logging of unknown settings frames

Posted by Rémy Maucherat <re...@apache.org>.
On Wed, Mar 23, 2022 at 10:01 PM Christopher Schultz
<ch...@christopherschultz.net> wrote:
>
> Rémy,
>
> On 3/23/22 16:10, remm@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch 10.0.x
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> > commit a82ddf0fc42c960f224e7d23eaa90df272de3559
> > Author: remm <re...@apache.org>
> > AuthorDate: Wed Mar 23 21:00:41 2022 +0100
> >
> >      PR #487: Improve logging of unknown settings frames
> >
> >      Pull request by Thomas Hoffmann.
> > ---
> >   java/org/apache/coyote/http2/ConnectionSettingsBase.java | 2 --
> >   java/org/apache/coyote/http2/Http2Parser.java            | 7 ++++++-
> >   java/org/apache/coyote/http2/Http2UpgradeHandler.java    | 7 ++++++-
> >   webapps/docs/changelog.xml                               | 4 ++++
> >   4 files changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/java/org/apache/coyote/http2/ConnectionSettingsBase.java b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> > index 042fb0c..ef4a200 100644
> > --- a/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> > +++ b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> > @@ -88,8 +88,6 @@ abstract class ConnectionSettingsBase<T extends Throwable> {
> >               break;
> >           case UNKNOWN:
> >               // Unrecognised. Ignore it.
> > -            log.warn(sm.getString("connectionSettings.unknown",
> > -                    connectionId, setting, Long.toString(value)));
> >               return;
> >           }
>
>
> Was it intended to remove this log completely?

Yes, there is not enough information to do the logging there.

Rémy

> -chris
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 01/02: PR #487: Improve logging of unknown settings frames

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Rémy,

On 3/23/22 16:10, remm@apache.org wrote:
> This is an automated email from the ASF dual-hosted git repository.
> 
> remm pushed a commit to branch 10.0.x
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
> 
> commit a82ddf0fc42c960f224e7d23eaa90df272de3559
> Author: remm <re...@apache.org>
> AuthorDate: Wed Mar 23 21:00:41 2022 +0100
> 
>      PR #487: Improve logging of unknown settings frames
>      
>      Pull request by Thomas Hoffmann.
> ---
>   java/org/apache/coyote/http2/ConnectionSettingsBase.java | 2 --
>   java/org/apache/coyote/http2/Http2Parser.java            | 7 ++++++-
>   java/org/apache/coyote/http2/Http2UpgradeHandler.java    | 7 ++++++-
>   webapps/docs/changelog.xml                               | 4 ++++
>   4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/java/org/apache/coyote/http2/ConnectionSettingsBase.java b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> index 042fb0c..ef4a200 100644
> --- a/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> +++ b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
> @@ -88,8 +88,6 @@ abstract class ConnectionSettingsBase<T extends Throwable> {
>               break;
>           case UNKNOWN:
>               // Unrecognised. Ignore it.
> -            log.warn(sm.getString("connectionSettings.unknown",
> -                    connectionId, setting, Long.toString(value)));
>               return;
>           }


Was it intended to remove this log completely?

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


[tomcat] 01/02: PR #487: Improve logging of unknown settings frames

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit a82ddf0fc42c960f224e7d23eaa90df272de3559
Author: remm <re...@apache.org>
AuthorDate: Wed Mar 23 21:00:41 2022 +0100

    PR #487: Improve logging of unknown settings frames
    
    Pull request by Thomas Hoffmann.
---
 java/org/apache/coyote/http2/ConnectionSettingsBase.java | 2 --
 java/org/apache/coyote/http2/Http2Parser.java            | 7 ++++++-
 java/org/apache/coyote/http2/Http2UpgradeHandler.java    | 7 ++++++-
 webapps/docs/changelog.xml                               | 4 ++++
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/coyote/http2/ConnectionSettingsBase.java b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
index 042fb0c..ef4a200 100644
--- a/java/org/apache/coyote/http2/ConnectionSettingsBase.java
+++ b/java/org/apache/coyote/http2/ConnectionSettingsBase.java
@@ -88,8 +88,6 @@ abstract class ConnectionSettingsBase<T extends Throwable> {
             break;
         case UNKNOWN:
             // Unrecognised. Ignore it.
-            log.warn(sm.getString("connectionSettings.unknown",
-                    connectionId, setting, Long.toString(value)));
             return;
         }
 
diff --git a/java/org/apache/coyote/http2/Http2Parser.java b/java/org/apache/coyote/http2/Http2Parser.java
index 5875e28..8c67d84 100644
--- a/java/org/apache/coyote/http2/Http2Parser.java
+++ b/java/org/apache/coyote/http2/Http2Parser.java
@@ -337,7 +337,12 @@ class Http2Parser {
                 }
                 int id = ByteUtil.getTwoBytes(setting, 0);
                 long value = ByteUtil.getFourBytes(setting, 2);
-                output.setting(Setting.valueOf(id), value);
+                Setting key = Setting.valueOf(id);
+                if (log.isDebugEnabled() && key == Setting.UNKNOWN) {
+                    log.warn(sm.getString("connectionSettings.unknown",
+                        connectionId, Integer.toString(id), Long.toString(value)));
+                }
+                output.setting(key, value);
             }
         }
         output.settingsEnd(ack);
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 91abf18..75bdac1 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -231,7 +231,12 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                 for (int i = 0; i < settings.length % 6; i++) {
                     int id = ByteUtil.getTwoBytes(settings, i * 6);
                     long value = ByteUtil.getFourBytes(settings, (i * 6) + 2);
-                    remoteSettings.set(Setting.valueOf(id), value);
+                    Setting key = Setting.valueOf(id);
+                    if (log.isDebugEnabled() && key == Setting.UNKNOWN) {
+                        log.warn(sm.getString("connectionSettings.unknown",
+                            connectionId, Integer.toString(id), Long.toString(value)));
+                    }
+                    remoteSettings.set(key, value);
                 }
             } catch (Http2Exception e) {
                 throw new ProtocolException(
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ef25e2d..90dde11 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -120,6 +120,10 @@
         skipping setting it in some cases (for example, it does not make
         sense for OpenSSL TLS 1.3). (remm)
       </fix>
+      <fix>
+        <pr>487</pr>: Improve logging of unknown settings frames. Pull request
+        by Thomas Hoffmann. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org