On Fri, 17 May 2024 at 00:11, Gary Gregory <garydgreg...@gmail.com> wrote: > > Can we PLEASE not do this unless we know what the plan is for Commons > overall? I really don't want to have this stuff copied in all Commons > Components because I doubt we will want to add Commons Lang as a dependency > in all Components.
Agreed. Also if we are to use annotations great care must be taken with the RetentionPolicy. Using the wrong setting can mean adding unnecessary runtime dependencies (this was an issue with the JCIP annotations @ThreadSafe etc) > So, what's the plan? Do you plan on copying this stuff > over and over or depending on Commons Lang all over. > > Imaging using code assit and seeing these types being offered from all over > the place... And finding they are all slightly different... > > Gary > > On Thu, May 16, 2024, 6:30 PM <joc...@apache.org> wrote: > > > This is an automated email from the ASF dual-hosted git repository. > > > > jochen pushed a commit to branch master > > in repository https://gitbox.apache.org/repos/asf/commons-lang.git > > > > > > The following commit(s) were added to refs/heads/master by this push: > > new 3322d9748 Adding the @Insecure, and @Safe annotations. > > 3322d9748 is described below > > > > commit 3322d974876b8d4f934d3544967103ebbcaef726 > > Author: Jochen Wiedmann <jochen.wiedm...@gmail.com> > > AuthorDate: Fri May 17 00:28:39 2024 +0200 > > > > Adding the @Insecure, and @Safe annotations. > > --- > > src/changes/changes.xml | 1 + > > .../apache/commons/lang3/annotations/Insecure.java | 48 ++++++++ > > .../org/apache/commons/lang3/annotations/Safe.java | 61 +++++++++++ > > .../commons/lang3/annotations/package-info.java | 37 +++++++ > > .../commons/lang3/annotations/AnnotationsTest.java | 122 > > +++++++++++++++++++++ > > 5 files changed, 269 insertions(+) > > > > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > > index 34841687a..b69e1f8a2 100644 > > --- a/src/changes/changes.xml > > +++ b/src/changes/changes.xml > > @@ -140,6 +140,7 @@ The <action> type attribute can be > > add,update,fix,remove. > > <action type="update" dev="ggregory" > > due-to="Dependabot">Bump org.apache.commons:commons-text from 1.11.0 to > > 1.12.0 #1200.</action> > > <!-- REMOVE --> > > <action type="remove" dev="ggregory" > > due-to="Paranoïd User">Drop obsolete JDK 13 Maven profile #1142.</action> > > + <action type="add" dev="jochen">Added the > > annotations package, including the Insecure, and Safe annotations.</action> > > </release> > > <release version="3.14.0" date="2023-11-18" description="New features > > and bug fixes (Java 8 or above)."> > > <!-- FIX --> > > diff --git > > a/src/main/java/org/apache/commons/lang3/annotations/Insecure.java > > b/src/main/java/org/apache/commons/lang3/annotations/Insecure.java > > new file mode 100644 > > index 000000000..2802f1189 > > --- /dev/null > > +++ b/src/main/java/org/apache/commons/lang3/annotations/Insecure.java > > @@ -0,0 +1,48 @@ > > +/* > > + * 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.commons.lang3.annotations; > > + > > +import java.lang.annotation.Documented; > > +import java.lang.annotation.ElementType; > > +import java.lang.annotation.Retention; > > +import java.lang.annotation.RetentionPolicy; > > +import java.lang.annotation.Target; > > + > > +/** > > + * This annotation is used to indicate, that a constructor, or method > > + * is insecure to use, unless the input parameters contain safe > > ("trusted") > > + * values. > > + * > > + * For example, consider a method like <pre> > > + * {@literal @Insecure} > > + * public void runCommand(String pCmdLine) { > > + * } > > + * </pre> > > + * > > + * The example method would invoke {@code /bin/sh} (Linux, Unix, or > > MacOS), or > > + * {@code cmd} (Windows) to run an external command, as given by the > > parameter > > + * {@code pCmdLine}. Obviously, depending on the value of the parameter, > > + * this can be dangerous, unless the API user (downstream developer) > > + * <em>knows</em>, that the parameter value is safe (for example, because > > it > > + * is hard coded, or because it has been compared to a white list of > > + * permissible values). > > + */ > > +@Retention(RetentionPolicy.RUNTIME) > > +@Target({ElementType.CONSTRUCTOR, ElementType.METHOD}) > > +@Documented > > +public @interface Insecure { > > +} > > diff --git a/src/main/java/org/apache/commons/lang3/annotations/Safe.java > > b/src/main/java/org/apache/commons/lang3/annotations/Safe.java > > new file mode 100644 > > index 000000000..4b5212c71 > > --- /dev/null > > +++ b/src/main/java/org/apache/commons/lang3/annotations/Safe.java > > @@ -0,0 +1,61 @@ > > +/* > > + * 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.commons.lang3.annotations; > > + > > +import java.lang.annotation.Documented; > > +import java.lang.annotation.ElementType; > > +import java.lang.annotation.Retention; > > +import java.lang.annotation.RetentionPolicy; > > +import java.lang.annotation.Target; > > + > > +/** > > + * This annotation is used to indicate, that a variable, field, or > > parameter > > + * contains a safe value. If so, the annotated element may be used in an > > + * invocation of a constructor, or method, which is annotated with > > + * {@code @Trusted}. > > + * > > + * For example, suggest the following method declaration: > > + * <pre> > > + * {@literal @Insecure} > > + * public void runCommand(String pCmdLine) { > > + * } > > + * </pre> > > + * > > + * Based on the example, this piece of source code would be invalid: > > + * <pre>{@code > > + * String cmdLine = "echo" + " " + "okay"; > > + * // It is unknown, whether the {@code cmdLine} variable contains a > > safe value. > > + * // Thus, the following should be considered dangerous: > > + * runCommand(cmdLine); > > + * }</pre> > > + * > > + * In the following example, however, the value of {@code cmdLine} is > > + * supposed to be safe, so it may be used when invoking the {@code > > runCommand} > > + * method. > > + * <pre> > > + * {@literal @Safe} String cmdLine = "echo" + " " + "okay"; > > + * // It is unknown, whether the {@code cmdLine} variable contains a > > safe value. > > + * // Thus, the following should be considered dangerous: > > + * runCommand(cmdLine); > > + * </pre> > > + */ > > +@Retention(RetentionPolicy.RUNTIME) > > +@Target({ElementType.LOCAL_VARIABLE, ElementType.FIELD, > > ElementType.PARAMETER}) > > +@Documented > > +public @interface Safe { > > + > > +} > > diff --git > > a/src/main/java/org/apache/commons/lang3/annotations/package-info.java > > b/src/main/java/org/apache/commons/lang3/annotations/package-info.java > > new file mode 100644 > > index 000000000..43d54d606 > > --- /dev/null > > +++ b/src/main/java/org/apache/commons/lang3/annotations/package-info.java > > @@ -0,0 +1,37 @@ > > +/* > > + * 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. > > + */ > > + > > +/** > > + * Provides annotations, that are designed to aim in static code analysis, > > + * and other areas of self-describing code. As of this writing, the > > following > > + * annotations are available: > > + * <dl> > > + * <dt>{@link Insecure}</dt> > > + * <dd>Indicates, that a constructor, method, or parameter should only > > + * take input, that can be considered as <em>safe</em>. > > + * The API user (the downstream developer) is supposed to ensure, by > > + * whatever means, that the input is safe, and doesn't trigger any > > + * security related issues.</dd> > > + * <dt>{@link Safe}</dt> > > + * <dd>By annotating a variable with {@code @Safe}, the API user > > + * declares, that the variable contains trusted input, that can be > > + * used as a parameter in an invocation of a constructor, or method, > > + * that is annotated with {@code @Trusted}.</dd> > > + * </dl> > > + * @since 3.15 > > + */ > > +package org.apache.commons.lang3.annotations; > > diff --git > > a/src/test/java/org/apache/commons/lang3/annotations/AnnotationsTest.java > > b/src/test/java/org/apache/commons/lang3/annotations/AnnotationsTest.java > > new file mode 100644 > > index 000000000..df1aa70f3 > > --- /dev/null > > +++ > > b/src/test/java/org/apache/commons/lang3/annotations/AnnotationsTest.java > > @@ -0,0 +1,122 @@ > > +/* > > + * 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.commons.lang3.annotations; > > + > > +import static org.junit.jupiter.api.Assertions.assertNotNull; > > +import static org.junit.jupiter.api.Assertions.assertSame; > > + > > +import java.lang.annotation.Target; > > +import java.util.function.Function; > > + > > +import org.junit.jupiter.api.Test; > > + > > + > > +/** This class ensures, that the annotations are properly configured > > + * with regard to {@link Target}. > > + * > > + * The so-called test methods are not actually testing anything, because > > + * an invalid configuration would be detected by the compiler. However, > > + * we have the unit test framework in place, and it is running anyways, > > + * so there's no harm in a few additional methods. > > + */ > > +public class AnnotationsTest { > > + public static class Wrapper { > > + private final Object wrappedObject; > > + > > + @Insecure > > + public Wrapper(Object wrappedObject) { > > + this.wrappedObject = wrappedObject; > > + } > > + > > + Object getWrappedObject() { > > + return wrappedObject; > > + } > > + } > > + > > + private static Wrapper newWrapper(Object wrappedObject) { > > + return new Wrapper(wrappedObject); > > + } > > + > > + /** Test, whether we can have an @Insecure annotation on a > > constructor. > > + */ > > + @Test > > + public void testConstructorAnnotatableAsInsecure() { > > + final Object unsafeObject = new Object(); > > + // Static code analysis should reject this, because the > > + // parameter (the newly created instance of Object) isn't known > > + // to be safe. > > + final Wrapper wrapper = new Wrapper(unsafeObject); > > + assertNotNull(wrapper); > > + assertSame(unsafeObject, wrapper.getWrappedObject()); > > + } > > + > > + /** Test, whether we can have an @Insecure annotation on a method. > > + */ > > + @Test > > + public void testMethodAnnotatableAsInsecure() { > > + final Object unsafeObject = new Object(); > > + // Static code analysis should reject this, because the > > + // parameter (the newly created instance of Object) isn't known > > + // to be safe. > > + final Wrapper wrapper = newWrapper(unsafeObject); > > + assertNotNull(wrapper); > > + assertSame(unsafeObject, wrapper.getWrappedObject()); > > + } > > + > > + /** Test, whether we can have a @Safe annotation on a local variable. > > + */ > > + @Test > > + public void testLocalVariablesAnnotatableAsSafe() { > > + @Safe final String wrappedString = "Hello, world!"; > > + // Static code analysis should accept this, because the variable > > + // is annotated with @Safe. > > + final Wrapper wrapper = newWrapper(wrappedString); > > + assertNotNull(wrapper); > > + assertSame(wrappedString, wrapper.getWrappedObject()); > > + } > > + > > + /** Test, whether we can have a @Safe annotation on a field. > > + */ > > + @Test > > + public void testFieldsAnnotatableAsSafe() { > > + // Static code analysis should accept this, because the field > > + // is annotated with @Safe. > > + final Wrapper wrapper = newWrapper(wrappedStringField); > > + assertNotNull(wrapper); > > + assertSame(wrappedStringField, wrapper.getWrappedObject()); > > + } > > + @Safe private String wrappedStringField = "Hello, world!"; > > + > > + /** Test, whether we can have a @Safe annotation on a field. > > + */ > > + @Test > > + public void testParametersAnnotatableAsSafe() { > > + // Static code analysis should accept this, because the parameter > > + // is annotated with @Safe. > > + final Function<String, Wrapper> wrapperCreator = > > + new Function<String, Wrapper>() { > > + @Override > > + public Wrapper apply(@Safe String wrappedObject) { > > + return newWrapper(wrappedObject); > > + } > > + }; > > + final String helloWorldString = "Hello, world!"; > > + final Wrapper wrapper = wrapperCreator.apply(helloWorldString); > > + assertNotNull(wrapper); > > + assertSame(helloWorldString, wrapper.getWrappedObject()); > > + } > > +} > > > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
