You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by bd...@apache.org on 2020/11/03 11:38:11 UTC

[sling-org-apache-sling-graphql-core] branch master updated (f769b0b -> fdb2fad)

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

bdelacretaz pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git.


    from f769b0b  Merge pull request #13 from jasghar/SLING-9870
     new fdfa9fa  SLING-9872 - use consistent naming
     new fdb2fad  SLING-9872 - sanitize logged values

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:
 .../graphql/core/engine/DefaultQueryExecutor.java  | 15 ++++--
 .../sling/graphql/core/logging/Sanitizer.java      | 56 ++++++++++++++++++++++
 .../graphql/core/schema/DefaultSchemaProvider.java |  4 +-
 .../SanitizerTest.java}                            | 51 +++++++++++---------
 4 files changed, 98 insertions(+), 28 deletions(-)
 create mode 100644 src/main/java/org/apache/sling/graphql/core/logging/Sanitizer.java
 copy src/test/java/org/apache/sling/graphql/core/{engine/SlingDataFetcherNameValidationTest.java => logging/SanitizerTest.java} (52%)


[sling-org-apache-sling-graphql-core] 02/02: SLING-9872 - sanitize logged values

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

bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git

commit fdb2fad3ecc153b278cd80560833213192827e08
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Nov 3 12:36:15 2020 +0100

    SLING-9872 - sanitize logged values
---
 .../graphql/core/engine/DefaultQueryExecutor.java  | 15 ++++-
 .../sling/graphql/core/logging/Sanitizer.java      | 56 +++++++++++++++++
 .../sling/graphql/core/logging/SanitizerTest.java  | 71 ++++++++++++++++++++++
 3 files changed, 139 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
index 76c815e..97506c0 100644
--- a/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
+++ b/src/main/java/org/apache/sling/graphql/core/engine/DefaultQueryExecutor.java
@@ -32,6 +32,7 @@ import org.apache.sling.graphql.api.SlingGraphQLException;
 import org.apache.sling.graphql.api.engine.QueryExecutor;
 import org.apache.sling.graphql.api.engine.ValidationResult;
 import org.apache.sling.graphql.api.SlingTypeResolver;
+import org.apache.sling.graphql.core.logging.Sanitizer;
 import org.apache.sling.graphql.core.scalars.SlingScalarsProvider;
 import org.apache.sling.graphql.core.schema.RankedSchemaProviders;
 import org.jetbrains.annotations.NotNull;
@@ -78,6 +79,8 @@ public class DefaultQueryExecutor implements QueryExecutor {
     public static final String RESOLVER_OPTIONS = "options";
     public static final String RESOLVER_SOURCE = "source";
 
+    private static final Sanitizer cleanLog = new Sanitizer();
+
     @Reference
     private RankedSchemaProviders schemaProvider;
 
@@ -130,7 +133,10 @@ public class DefaultQueryExecutor implements QueryExecutor {
             LOGGER.debug("Resource {} maps to GQL schema {}", queryResource.getPath(), schemaDef);
             final GraphQLSchema schema = buildSchema(schemaDef, queryResource);
             final GraphQL graphQL = GraphQL.newGraphQL(schema).build();
-            LOGGER.debug("Executing query\n[{}]\nat [{}] with variables [{}]", query, queryResource.getPath(), variables);
+            if(LOGGER.isDebugEnabled()) {
+                LOGGER.debug("Executing query\n[{}]\nat [{}] with variables [{}]",
+                    cleanLog.sanitize(query), queryResource.getPath(), cleanLog.sanitize(variables.toString()));
+            }
             ExecutionInput ei = ExecutionInput.newExecutionInput()
                     .query(query)
                     .variables(variables)
@@ -146,8 +152,11 @@ public class DefaultQueryExecutor implements QueryExecutor {
                         }
                     }
                 }
-                LOGGER.error("Query failed for Resource {}: query={} Errors:{}, schema={}",
-                        queryResource.getPath(), query, errors, schemaDef);
+                if(LOGGER.isErrorEnabled()) {
+                    LOGGER.error("Query failed for Resource {}: query={} Errors:{}, schema={}",
+                        queryResource.getPath(), cleanLog.sanitize(query), errors, schemaDef);
+                }
+
             }
             LOGGER.debug("ExecutionResult.isDataPresent={}", result.isDataPresent());
             return result.toSpecification();
diff --git a/src/main/java/org/apache/sling/graphql/core/logging/Sanitizer.java b/src/main/java/org/apache/sling/graphql/core/logging/Sanitizer.java
new file mode 100644
index 0000000..47d7641
--- /dev/null
+++ b/src/main/java/org/apache/sling/graphql/core/logging/Sanitizer.java
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.sling.graphql.core.logging;
+
+import java.text.CharacterIterator;
+import java.text.StringCharacterIterator;
+
+/** Sanitize strings for logging */
+public class Sanitizer {
+
+    /** Character that replaces unwanted ones in input */
+    public static final char REPLACEMENT_CHAR = '_';
+
+    public String sanitize(String input) {
+        if(input == null) {
+            return null;
+        }
+        final StringBuilder result = new StringBuilder(input.length());
+        final StringCharacterIterator it = new StringCharacterIterator(input);
+        for(char c = it.first(); c != CharacterIterator.DONE; c = it.next()) {
+            result.append(accept(c) ? c : REPLACEMENT_CHAR);
+        }
+        return result.toString();
+    }
+
+    /**  Reject non US-ASCII characters as well as
+     *  control chars which are not end-of-line chars.
+     */
+    private static boolean accept(char c) {
+        if(c > 0x7f) {
+            return false;
+        } else if(c == '\n' || c == '\r')  {
+            return true;
+        } else if(Character.isISOControl(c)) {
+            return false;
+        }
+        return true;
+    }
+}
diff --git a/src/test/java/org/apache/sling/graphql/core/logging/SanitizerTest.java b/src/test/java/org/apache/sling/graphql/core/logging/SanitizerTest.java
new file mode 100644
index 0000000..90553c2
--- /dev/null
+++ b/src/test/java/org/apache/sling/graphql/core/logging/SanitizerTest.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.graphql.core.logging;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class SanitizerTest {
+    private static final String UNCHANGED = "\u0451";
+    private final Sanitizer s = new Sanitizer();
+    private final String in;
+    private final String out;
+
+    @Parameters(name="{0}")
+    public static Collection<Object[]> data() {
+        final List<Object[]> result = new ArrayList<>();
+        testCase(result, null, UNCHANGED);
+        testCase(result, "nochange", UNCHANGED);
+        testCase(result, "end\r\nof line", UNCHANGED);
+        testCase(result, "with\ttab", "with_tab");
+        testCase(result, "The quick brown fox jumps over the lazy dog!", UNCHANGED);
+        testCase(result, "non-ASCII \u0041 z \u0080 \u0441", "non-ASCII A z _ _");
+        testCase(result, "quotes \"ok\" and 'single'", UNCHANGED);
+        testCase(result, "paren(theses)", UNCHANGED);
+        testCase(result, "@fetcher('something')", UNCHANGED);
+        return result;
+    }
+
+    private static void testCase(List<Object[]> result, String in, String out) {
+        result.add(new Object[] { in, out});
+    }
+
+    public SanitizerTest(String in, String out) {
+        this.in = in;
+        this.out = out;
+    }
+
+    @Test
+    public void verify() {
+        if(out.equals(UNCHANGED)) {
+            assertEquals(in, s.sanitize(in));
+        } else {
+            assertEquals(out, s.sanitize(in));
+        }
+    }
+}


[sling-org-apache-sling-graphql-core] 01/02: SLING-9872 - use consistent naming

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

bdelacretaz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-graphql-core.git

commit fdfa9fa7b335a112aeb3122785d3c1aa7c57377d
Author: Bertrand Delacretaz <bd...@apache.org>
AuthorDate: Tue Nov 3 10:42:12 2020 +0100

    SLING-9872 - use consistent naming
---
 .../org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java b/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
index bcf7bf8..2574303 100644
--- a/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
+++ b/src/main/java/org/apache/sling/graphql/core/schema/DefaultSchemaProvider.java
@@ -42,7 +42,7 @@ import org.slf4j.LoggerFactory;
         Constants.SERVICE_VENDOR + "=The Apache Software Foundation" })
 public class DefaultSchemaProvider implements SchemaProvider {
 
-    private final Logger log = LoggerFactory.getLogger(getClass());
+    private final Logger LOGGER = LoggerFactory.getLogger(getClass());
 
     public static final int SERVICE_RANKING =  Integer.MIN_VALUE + 1000;
     public static final String SCHEMA_EXTENSION = "GQLschema";
@@ -59,7 +59,7 @@ public class DefaultSchemaProvider implements SchemaProvider {
             .withExtension(SCHEMA_EXTENSION)
         ;
 
-        log.debug("Getting GraphQL Schema for {}: {}", r.getPath(), req);
+        LOGGER.debug("Getting GraphQL Schema for {}: {}", r.getPath(), req);
 
         if(req.execute().getStatus() == HttpServletResponse.SC_OK) {
             return req.getResponseAsString();