diff --git a/src/site/antora/modules/ROOT/pages/manual/api.adoc b/src/site/antora/modules/ROOT/pages/manual/api.adoc index 525c5d54e0e..732ea5096a1 100644 --- a/src/site/antora/modules/ROOT/pages/manual/api.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/api.adoc @@ -78,6 +78,8 @@ include::partial$manual/api-best-practice-exception-as-last-argument.adoc[] include::partial$manual/api-best-practice-dont-use-string-concat.adoc[] +include::partial$manual/api-best-practice-dont-mix-concat-and-params.adoc[] + [#best-practice-supplier] === Use ``Supplier``s to pass computationally expensive arguments diff --git a/src/site/antora/modules/ROOT/pages/manual/getting-started.adoc b/src/site/antora/modules/ROOT/pages/manual/getting-started.adoc index 9b6da47430d..19c8a2d7204 100644 --- a/src/site/antora/modules/ROOT/pages/manual/getting-started.adoc +++ b/src/site/antora/modules/ROOT/pages/manual/getting-started.adoc @@ -121,6 +121,8 @@ include::partial$manual/api-best-practice-exception-as-last-argument.adoc[] include::partial$manual/api-best-practice-dont-use-string-concat.adoc[] +include::partial$manual/api-best-practice-dont-mix-concat-and-params.adoc[] + [#install-app] == How do I install Log4j Core to run my **application**? diff --git a/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-mix-concat-and-params.adoc b/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-mix-concat-and-params.adoc new file mode 100644 index 00000000000..ca53a7eba03 --- /dev/null +++ b/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-mix-concat-and-params.adoc @@ -0,0 +1,34 @@ +//// + 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. +//// + +If you are mixing `String` concatenation and parameterized logging, you are doing something very wrong and dangerous! + +* [ ] The format string of a parameterized statement should be a compile time constant! +An attacker could mangle your logs by inserting `{}` placeholders in the values! +Try these examples with `userId="{}\nbadUser"` and `reason="root logged in successfully"` ++ +[source,java] +---- +/* BAD! */ LOGGER.info("User " + userId + " logged out: {}", reason); +---- + +* [x] Use message parameters ++ +[source,java] +---- +/* GOOD */ LOGGER.info("User {} logged out: {}", userId, reason); +---- diff --git a/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-use-string-concat.adoc b/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-use-string-concat.adoc index 5759d12dc94..82ccf6657c2 100644 --- a/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-use-string-concat.adoc +++ b/src/site/antora/modules/ROOT/partials/manual/api-best-practice-dont-use-string-concat.adoc @@ -15,13 +15,8 @@ limitations under the License. //// -If you are using `String` concatenation while logging, you are doing something very wrong and dangerous! - -* [ ] Don't use `String` concatenation to format arguments! -This circumvents the handling of arguments by message type and layout. -More importantly, **this approach is prone to attacks!** -Imagine `userId` being provided by the user with the following content: -`placeholders for non-existing args to trigger failure: {} {} \{dangerousLookup}` +* [ ] Don't use `String` concatenation to format arguments: +the log message will be formatted, even if the logger is not enabled, and you will suffer a performance penalty! + [source,java] ---- @@ -34,3 +29,10 @@ Imagine `userId` being provided by the user with the following content: ---- /* GOOD */ LOGGER.info("failed for user ID `{}`", userId); ---- + +* [x] Use message lambdas ++ +[source,java] +---- +/* GOOD */ LOGGER.info(() -> "failed for user ID: " + userId); +----