jdaugherty commented on code in PR #16631: URL: https://github.com/apache/grails-core/pull/16631#discussion_r4367908555
########## grails-data-mongodb/core/src/main/groovy/org/apache/grails/data/mongo/core/querytail/MongoCommandRedactor.groovy: ########## @@ -0,0 +1,241 @@ +/* + * 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 + * + * https://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.grails.data.mongo.core.querytail + +import groovy.transform.CompileStatic +import org.bson.BsonDocument +import org.bson.BsonValue + +import org.apache.grails.data.core.querytail.QueryTailRedactor + +/** + * Takes the values out of a MongoDB command before it is captured. A command is a document, so + * there is nothing to parse: every value becomes {@link QueryTailRedactor#PLACEHOLDER}, and the + * keys, which are the options of the command, its operators and the field names of the collection, + * are kept, except where a key may be data. The keys of a document stored in a map property, or in + * a property or a collection the {@linkplain MongoMappedFields mapping} does not know, are recorded + * as the placeholder too, and so is every segment of a dotted path past such a key. A numeric key + * is kept only where it indexes an array the mapping knows; in a map, or at the top of a document, + * it is data. The entries the driver adds for its session, the cluster time and the transaction, + * which say nothing about the command, are left out. + * + * <p>A command is recorded as its name, the database and collection it runs against, and the + * document of its remaining entries: {@code find shop.book {"filter": {"title": ?}, "limit": ?}}.</p> + * + * @since 8.1 + */ +@CompileStatic +final class MongoCommandRedactor { + + private static final String PLACEHOLDER = QueryTailRedactor.PLACEHOLDER + + private static final String DATABASE = '$db' + + private static final String COLLECTION = 'collection' + + /** The entries the driver adds, which describe its session and transaction rather than the command. */ + private static final Set<String> LEFT_OUT = ['lsid', '$clusterTime', 'txnNumber', 'autocommit', + 'startTransaction', '$readPreference', 'apiVersion', 'apiStrict', 'apiDeprecationErrors'] as Set<String> + + /** + * The options whose document, or documents, are keyed by the fields of the collection, wherever they appear, + * including those of the operations of a client-level {@code bulkWrite}. + */ + private static final Set<String> FIELD_KEYED = ['filter', 'query', 'q', 'u', 'update', 'document', 'documents', + 'updateMods', 'pipeline', 'sort', 'projection', 'fields', 'key', 'hint', 'min', 'max', 'arrayFilters', + 'partialFilterExpression', 'wildcardProjection'] as Set<String> + + private MongoCommandRedactor() { + } + + /** + * Redacts a command. + * + * @param database the database the command runs against + * @param commandName the name of the command, which is its first key + * @param command the command as the driver sends it + * @param fields the fields the mapping of the datastore knows, or null before the datastore is built + * @return the command with its values replaced + */ + static String redact(String database, String commandName, BsonDocument command, MongoMappedFields fields) { + String collection = null + String collectionKey = null + BsonValue first = command.get(commandName) + if (first != null && first.isString()) { + collection = first.asString().value + collectionKey = commandName + } + else if (command.get(COLLECTION)?.isString()) { + collection = command.getString(COLLECTION).value + collectionKey = COLLECTION + } + StringBuilder out = new StringBuilder(commandName) + if (database != null || collection != null) { + out.append(' ') + if (database != null) { + out.append(database) + } + if (collection != null) { + out.append(database != null ? '.' : '').append(collection) + } + } + Scope options = Scope.options(collection, fields) + StringBuilder body = new StringBuilder() + for (Map.Entry<String, BsonValue> entry in command.entrySet()) { + String key = entry.key + if (key == DATABASE || key == collectionKey || LEFT_OUT.contains(key)) { + continue + } + body.append(body.length() == 0 ? '{' : ', ') + appendEntry(body, key, entry.value, options) + } + if (body.length() > 0) { + out.append(' ').append(body).append('}') + } + out.toString() + } + + private static void appendEntry(StringBuilder out, String key, BsonValue value, Scope scope) { + Scope child = scope.of(key) + out.append(child.keyText).append(': ') + appendValue(out, value, child) + } + + private static void appendValue(StringBuilder out, BsonValue value, Scope scope) { + if (value.isDocument()) { + out.append('{') + boolean first = true + for (Map.Entry<String, BsonValue> entry in value.asDocument().entrySet()) { + if (!first) { + out.append(', ') + } + first = false + appendEntry(out, entry.key, entry.value, scope) + } + out.append('}') + } + else if (value.isArray()) { + out.append('[') + boolean first = true + for (BsonValue element in value.asArray()) { + if (!first) { + out.append(', ') + } + first = false + appendValue(out, element, scope) Review Comment: Fixed in b0e9a8c9a4 (folded into the feature commit). The scope now advances into an array element: `Scope` carries a `depth`, the number of indices taken into the field at its path, an array element gets `depth + 1`, and so does the document of an operator on an array such as `$elemMatch`. A numeric key is kept only when `MongoMappedFields.isIndexed(collection, path, depth)` says the field is an array at that depth: at depth 0 an array- or `Collection`-typed property, at depth 1 only when its element type (the array component type, or the `Basic` property's component type) is itself an array or collection, deeper never. So `attributes: [{"9876543210": "v"}]`, `attributes.0.9876543210`, `attributes: {$elemMatch: {"9876543210": 1}}` and a numeric key in an element of an embedded list all record `?`, while `stops.0.city` and `tags.0` keep their index. Covered in `MongoCommandRedactorSpec` (insert, dotted, `$elemMatch` and nested-array forms, plus the never-survives case with a numer ic secret in a list of maps) and through the capture API in `QueryTailCommandListenerSpec` against a real mongod, which saves a book whose `List<Map>` property holds a numeric key, queries `attributes.0.<key>`, and asserts the key reaches neither the insert nor the find in the file. The guide says a numeric key inside an element of such a list is data unless the property is a list of lists. ########## grails-datamapping-core/src/main/groovy/org/apache/grails/data/core/querytail/QueryTailRequestAttribution.groovy: ########## @@ -0,0 +1,361 @@ +/* + * 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 + * + * https://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.grails.data.core.querytail + +import java.lang.reflect.Method +import java.nio.charset.StandardCharsets +import java.util.concurrent.atomic.AtomicLong +import java.util.regex.Pattern + +import groovy.transform.CompileStatic + +import org.springframework.util.ClassUtils +import org.springframework.web.context.request.RequestAttributes +import org.springframework.web.context.request.RequestContextHolder +import org.springframework.web.context.request.ServletRequestAttributes + +/** + * Attributes a captured statement to the web request on the current thread. Grails keeps the + * request, and the controller and action it resolved, on the thread that serves it, so a statement + * run by an interceptor, the action or the view all land on the same page. The first statement of + * a request gives it an id that every later statement of that request repeats, which is how the + * {@code query-tail} command counts statements per request. Work on any other thread is attributed to + * the thread by name. + * + * <p>The request is recorded as its method and path, with the values in the path replaced by + * {@code ?}: the values of the parameters the URL mapping took from it and those of its query, found + * whether or not the client percent-encoded them, and any segment that is a number, a UUID or a long + * hexadecimal token, so that an id, a token or a key carried in a path never reaches the capture + * file.</p> + * + * <p>Spring Web is optional: without it on the classpath every statement is attributed to its + * thread.</p> + * + * @since 8.1 + */ +@CompileStatic +class QueryTailRequestAttribution { + + /** The request attribute holding the id this attribution gave a request. */ + public static final String REQUEST_ID_ATTRIBUTE = 'org.apache.grails.data.core.querytail.REQUEST_ID' + + /** The request attribute Grails sets the controller name in. */ + public static final String CONTROLLER_NAME_ATTRIBUTE = 'org.grails.CONTROLLER_NAME_ATTRIBUTE' + + /** The request attribute Grails sets the controller namespace in. */ + public static final String CONTROLLER_NAMESPACE_ATTRIBUTE = 'org.grails.CONTROLLER_NAMESPACE_ATTRIBUTE' + + /** The request attribute Grails sets the action name in. */ + public static final String ACTION_NAME_ATTRIBUTE = 'org.grails.ACTION_NAME_ATTRIBUTE' + + private static final boolean SPRING_WEB_PRESENT = ClassUtils.isPresent( + 'org.springframework.web.context.request.RequestContextHolder', QueryTailRequestAttribution.classLoader) + + private final AtomicLong requestCounter = new AtomicLong() + private final String requestIdPrefix + + /** + * An attribution whose request ids are {@code r1}, {@code r2} and so on. + */ + QueryTailRequestAttribution() { + this('') + } + + /** + * @param requestIdPrefix what every request id starts with, which tells the requests of one JVM + * from those of another writing the same capture + */ + QueryTailRequestAttribution(String requestIdPrefix) { + this.requestIdPrefix = requestIdPrefix ?: '' + } + + /** + * Fills in the request id, page and request of a record from the current thread. + * + * @param record the record of a statement that just ran on this thread + * @return the same record + */ + QueryTailRecord attribute(QueryTailRecord record) { + if (SPRING_WEB_PRESENT && WebRequestLookup.attribute(record, requestCounter, requestIdPrefix)) { + return record + } + record.requestId = null + record.request = null + record.page = QueryTailRecord.THREAD_PAGE_PREFIX + Thread.currentThread().name + record + } + + /** + * Keeps every reference to Spring Web, in a signature as well as in a body, in a class that is only + * loaded once Spring Web is known to be present. The outer class can then be loaded, and reflected + * on, without Spring Web on the classpath. + */ + @CompileStatic + private static class WebRequestLookup { + + private static final String REQUEST_ID_PREFIX = 'r' + + private static final String PLACEHOLDER = '?' + + /** The parameters a URL mapping sets that name code rather than carry a value. */ + private static final Set<String> MAPPING_PARAMETERS = ['controller', 'action', 'namespace', 'plugin', 'format'] as Set<String> + + /** A segment that is a number, a UUID or a long hexadecimal token is a value whatever the mapping says. */ + private static final Pattern VALUE_SHAPED = Pattern.compile( + '(?i)[0-9]+|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[0-9a-f]{32,}') + + private static final boolean SERVLET_PRESENT = ClassUtils.isPresent( + 'jakarta.servlet.http.HttpServletRequest', WebRequestLookup.classLoader) + + /** The {@code getParams()} of a Grails web request, which holds the parameters its URL mapping took, by class. */ + private static final ClassValue<Method> PARAMETERS = new ClassValue<Method>() { + @Override + protected Method computeValue(Class<?> type) { + try { + return type.getMethod('getParams') + } + catch (NoSuchMethodException ignored) { + return null + } + } + } + + /** + * Fills in a record from the request on the current thread. + * + * @param record the record to fill in + * @param requestCounter the counter the next request id is taken from + * @param requestIdPrefix what every request id starts with + * @return whether there is a request on the current thread + */ + static boolean attribute(QueryTailRecord record, AtomicLong requestCounter, String requestIdPrefix) { + RequestAttributes attributes = RequestContextHolder.getRequestAttributes() + if (attributes == null) { + return false + } + record.requestId = requestId(attributes, requestCounter, requestIdPrefix) + record.page = page(attributes) + record.request = SERVLET_PRESENT ? ServletRequestLookup.describe(attributes) : null + true + } + + /** + * Builds the request line: the method and the path, with the values in the path, the parameters + * the URL mapping took from it among them, replaced. + * + * @param method the HTTP method + * @param path the request path + * @param attributes the request, whose mapped parameters are read when it is a Grails web request + * @param parameters the parameters of the servlet request, by name, each a {@code String[]} + */ + static String describe(String method, String path, RequestAttributes attributes, Map<?, ?> parameters) { + Set<String> values = new HashSet<>() + addValues(values, mappedParameters(attributes)) + addValues(values, parameters) + "${method} ${redactPath(path, values)}".toString() + } + + private static String requestId(RequestAttributes attributes, AtomicLong requestCounter, String requestIdPrefix) { + synchronized (attributes) { + Object existing = attributes.getAttribute(REQUEST_ID_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST) + if (existing != null) { + return existing.toString() + } + String id = requestIdPrefix + REQUEST_ID_PREFIX + requestCounter.incrementAndGet() + attributes.setAttribute(REQUEST_ID_ATTRIBUTE, id, RequestAttributes.SCOPE_REQUEST) + return id + } + } + + private static String page(RequestAttributes attributes) { + Object controller = attributes.getAttribute(CONTROLLER_NAME_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST) + if (controller == null) { + return null + } + Object namespace = attributes.getAttribute(CONTROLLER_NAMESPACE_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST) + Object action = attributes.getAttribute(ACTION_NAME_ATTRIBUTE, RequestAttributes.SCOPE_REQUEST) + StringBuilder page = new StringBuilder() + if (namespace != null) { + page.append(namespace).append(':') + } + page.append(controller) + if (action != null) { + page.append('/').append(action) + } + page.toString() + } + + /** + * @return the parameters of a Grails web request, which include those its URL mapping took from + * the path, or null for a request that is not one + */ + private static Map<?, ?> mappedParameters(RequestAttributes attributes) { + Method parameters = PARAMETERS.get(attributes.getClass()) + if (parameters == null) { + return null + } + try { + Object value = parameters.invoke(attributes) + return value instanceof Map ? (Map<?, ?>) value : null + } + catch (ReflectiveOperationException | RuntimeException ignored) { + return null + } + } + + private static void addValues(Set<String> values, Map<?, ?> parameters) { + if (parameters == null) { + return + } + for (Map.Entry<?, ?> entry in parameters.entrySet()) { + if (MAPPING_PARAMETERS.contains(String.valueOf(entry.key))) { + continue + } + Object value = entry.value + if (value instanceof CharSequence) { + values.add(value.toString()) + } + else if (value instanceof Object[]) { + for (Object element in (Object[]) value) { + if (element instanceof CharSequence) { + values.add(element.toString()) + } + } + } + } + } + + /** + * Replaces the values in a path: a segment that is one of the values, with or without an + * extension, a run of segments that spells a value holding slashes, and any segment shaped + * like a value. The segments are matched as the client sent them and percent-decoded, since + * the parameters are decoded while the request URI is not; the path is written as sent. + */ + private static String redactPath(String path, Set<String> values) { + String[] segments = path.split('/', -1) + String[] decoded = new String[segments.length] + boolean[] redacted = new boolean[segments.length] + for (int i = 0; i < segments.length; i++) { Review Comment: Fixed in b0e9a8c9a4. Until the URL mapping has run, no segment's role is known, so the request line records only the shape of the path: `describe` treats the mapping as run when the controller attribute is set or the Grails request's parameters carry `controller`, and otherwise replaces every segment, so a statement run before the controller is chosen, or in a request that never reaches one, is written as `GET /?/?/?`; once the mapping has run the later statements of the same request get the mapped line with the literal segments kept. `QueryTailRequestAttributionSpec` records a statement on `/password/reset/k9fQ2x` before and after the mapping (`GET /?/?/?`, then `GET /password/reset/?`), and the rows that relied on the numeric, UUID and hex patterns now run with a mapped request, plus one unmapped row. The guide and the class javadoc say so, and the capture specs that assert a path now bind a controller. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
