The test started breaking on GitHub, but ran fine locally, which implied
some kind of classpath ordering issue. As it turned out, for XML tests
all JARs found in the sandbox directory are added to the classpath
automatically. So if we do not want them on the classpath, we need to
delete or rename them before running.
In this case, however, it was enough to make sure the classpath order is
correct, so that the duplicate 'Application' class is found in the woven
version, not in the unwoven one by chance. We therefore need to use the
correct syntax,
1. separating classpath entries by comma, not space,
2. prepending '$sandbox/' to the JAR name.
Now the test reliably does what it should have done from the start: run
the woven code. Until now, the output assertions were adjusted to the
weird result of no aspect logs being present on the console, which was
wrong right from the start.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix indentation of compilation results on the console
Whenever warnings or errors were printed via CompilationResult.toString,
indirectly also using MessageUtil.renderMessage(IMessage, boolean),
messages containing context info such as code snippets with carets
marking erroneous tokens - see also the previous commit - prefixes like
"[warning 1] warning at " were printed right in front of the code
snippets. I.e., the carets marking erroneous tokens in the second line
were not indented like the first line with the code snippet, leading to
(simplified) output like:
Especially with longer, more complex context lines, this helps to
identify the erroneous section. BTW, for one-line messages, everything
of course looks like before.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Bugfix: caret error marks in compiler output too short
This very old bug in EclipseAdapterUtils calculated the '^' caret error
marks incorrectly. The marks were too short like this:
void m() { return vax[3]; }
^^^^^^^^^^^
Where the correct result would look like this:
void m() { return vax[3]; }
^^^^^^^^^^^^^^
This was due to the rather complicated way in which first surrounding
context code - here the leading 'void m() { ' and trailing ' }' - was
first added and then possible leading white space was cut off again from
the left. It is difficult to explain, the code is a nightmare, trying to
work with char arrays, counting indexes, repeatedly using
System.arraycopy and using lots of counters and offsets. I would have
liked to simplify the code, converting char[] buffers to Strings, but
decided to keep the basic structure for now, not sure what kind of
memory or performance considerations led to this design.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Comment on newly pointcut parsing found problem in test class
A pointcut like
execution(*..Collection<?>[] *(..))
leads to an AJ core dump, which is something I noticed while fixing a
test in the previous commit. I am going to create a new issue for it and
link to this commit later.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
After the changes, the weaver's behaviour is now slightly different, but
not actually buggy. Actually, in one case there is now a weaver warning
for a non-matching pointcut which should have been there before, but was
not. I.e., things have improved and the tests are even a bit better now.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix PointcutRewriterTest, add LogicalPointcutStructure test helper class
After WildTypePattern.hashCode was fixed in the previous commit,
PointcutRewriterTest started failing, because in many places it was
falsely relying on a specific order of hash codes, which cannot be
guaranteed, especially since more instance fields are part of the hash
code now in accordance with 'equals'.
The new test helper class LogicalPointcutStructure is able to recognise
chained '&&' and '||' pointcuts of the same logical nesting level,
un-nesting them from the actual pointcut structure and making them
comparable, disregarding their order. I.e., something like
((A && B) && C) && D
is actually recognised to logically be
A && B && C && D
and equivalent to e.g. either of
D && B && A && C
A && B && D && C
C && A && D && B
This helps to compare rewritten pointcuts, as long as their logical
structure has not been altered.
Relates to #24.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
WildTypePattern: fix hashCode and toString methods
Especially 'hashCode' did not correspond to 'equals', disregarding
several fields, array dimension information being only one of them. This
led to parts of pointcuts being ignored, because they were regarded as
duplicates. Example:
execution(Foo* *(..)) && !execution(Foo*[] *(..))
Here, the negated pattern was falsely regarded as equal to the first
pattern, leading to an "A && !A" situation, i.e. no match at all.
Furthermore, 'toString' did not print array strings, i.e. instead of
"Foo*[][]" something like "Foo*" was printed. This false information was
also present in annotations generated by the weaver.
FuzzilyMatchingAspect was adjusted to actually match exactly once, as
expected, for the "Foo*" return types, i.e. exclusions for the array
return types have been added.
Relates to #24.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Handle one- and multi-dimensional array return types correctly
Fixes https://github.com/eclipse/org.aspectj/issues/24, both the array
return type matching as such as well as matching dimensionality patterns
correctly. E.g., 'Foo*[]' is not the same as 'Foo*[][]'. This also works
correctly in combination with asterisks, even for primitive types, i.e.
'in*[][]' correctly matches a 2-dimensional array of 'int'.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Refactor, comment and reactivate Ajc183Tests.testAnnoStyleDecp_442425
This test was always inactive and did not add any value other than
during development. No it runs, passes and documents the status quo of
- what was fixed (regression for AJC 1.8.2 core dump fixed in 1.8.3),
- the current limitations of @DeclareParents and @DeclareMixin
regarding generic interfaces.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
GitHub CI build: upgrade GabrielBB/xvfb-action to 1.6
CI builds started showing warnings as follows:
Node.js 12 actions are deprecated. For more information see:
https://github.blog/changelog/2022-09-22-github-actions-all-actions-will-begin-running-on-node16-instead-of-node12/.
Please update the following actions to use Node.js 16:
GabrielBB/xvfb-action@v1
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
This commit is a follow-up for 65f1ec72. The SOURCE retention case is
documented now and considered in a few more call sites. The
previously already similar code structures in
- DeclareAnnotation.ensureAnnotationDiscovered,
- DeclareAnnotation.getAnnotationType
have both been streamlined and still remain logically in sync.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
BCEL: use MAX_CP_ENTRIES from internal class, not from JRE
Instead of importing com.sun.org.apache.bcel.internal.Const, use
use org.aspectj.apache.bcel.Constants. The former class is from the
internal JRE module 'java.xml' which is not exposed by default.
Actually, no existing test failed because of it, but javadoc generation
for the AspectJ weaver.
Relates to #192.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Fix #366085 concerning declared annotations with source retention
See https://bugs.eclipse.org/bugs/show_bug.cgi?id=366085.
See https://stackoverflow.com/q/74618269/1082681.
The issue described in the Bugzilla issue is about 'declare @type', but
similar issues also existed for 'declare @field', 'declare @method',
'declare @constructor'. This fix is rather superficial and leaves
things to be desired, because it is rather hacky and simply ignores
errors source retention annotation declarations during weaving. A better
fix would drop the corresponding declarations while parsing and also
issue compiler warnings in each case.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
See https://github.com/advisories/GHSA-97xg-phpr-rg8q.
See https://issues.apache.org/jira/browse/BCEL-363.
See https://github.com/apache/commons-bcel/pull/147.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
OutputSpec.matchesThisVm better supports version ranges
vmVersionRanges might be a single version like "9", a list of versions
like "1.2,1.3,1.4,1.5", an equivalent range of "1.2-1.5", an open range
like "-1.8", "9-" (equivalent to "9+") or a more complex list of ranges
like "-1.6,9-11,13-14,17-" or "8,11,16+".
Empty ranges like in "", " ", "8,,14", ",5", "6-," will be ignored.
I.e., they will not yield a positive match.
Bogus ranges like "9-11-14" will be ignored, too.
Existing XML test specs using '<line text="..." vm="v,v2,...,vn"/>
have been adjusted to use version ranges rather than long lists of
consecutive versions. Furthermore, ranges with a trailing '+' like '14+'
were replaced by using the new canonical format '14-', even though the
parser still correctly recognises '14+'.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
For now, only the "pattern matching for switch" tests from previews 1
and 2 were adjusted to work in preview 3, because guarded patterns were
replaced by 'when' clauses in 'switch' blocks. Therefore, existing test
classes did not compile anymore and had to be replaced by syntactically
upgraded versions with content merged from preview 1 and 2 classes.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Firstly, Batik is not needed for running AspectJ tests.
Secondly, the fixed Windows path separators led to GitHub CI/CD tests
failing under Java 18+ on GitHub. Replacing ';' by 'File.pathSeparator'
would have fixed the problem, but removing the paths altogether is the
cleaner solution.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Add docs/dist/LICENSE-AspectJ.html to all public artifacts
The license file will be included right in the main directory of both
binary and source artifacts for
- AspectJ Matcher,
- AspectJ Runtime,
- AspectJ Weaver,
- AspectJ Tools (Compiler),
- AspectJ Installer.
Fixes #185.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
This fixes a partial path traversal vulnerability.
Replaces `dir.getCanonicalPath().startsWith(parent.getCanonicalPath())`, which is vulnerable to partial path traversal attacks, with the more secure `dir.getCanonicalFile().toPath().startsWith(parent.getCanonicalFile().toPath())`.
To demonstrate this vulnerability, consider `"/usr/outnot".startsWith("/usr/out")`.
The check is bypassed although `/outnot` is not under the `/out` directory.
It's important to understand that the terminating slash may be removed when using various `String` representations of the `File` object.
For example, on Linux, `println(new File("/var"))` will print `/var`, but `println(new File("/var", "/")` will print `/var/`;
however, `println(new File("/var", "/").getCanonicalPath())` will print `/var`.
Weakness: CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Severity: Medium
CVSSS: 6.1
Detection: CodeQL & OpenRewrite (https://public.moderne.io/recipes/org.openrewrite.java.security.PartialPathTraversalVulnerability)
Reported-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com> Signed-off-by: Jonathan Leitschuh <Jonathan.Leitschuh@gmail.com>
Bug-tracker: https://github.com/JLLeitschuh/security-research/issues/13
In order to create the docs separately, running just a single module,
add 'install' to the previous build step, so the 'docs' module can find
the 'lib' dummy JAR in the local Maven repo.
Relates to #150.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
I have not used those directories or any files inside them at all. I
hope I am not taking away anything helpful from people who prefer
working with inter-linked Eclipse projects instead of Maven, but I think
we will notice if anyone complains and can revert in case of an
emergency.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Remove Aspectj.dev artifact and plugin repositories
After having removed the custom-built Maven Shade version, there should
be no more references to artifacts in that repository left. Let us see
if the build passes.
Furthermore, since being a committer, I can also attach AspectJ
installers to GitHub releases.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
Bump Maven Shade Plugin to 3.3.0, add TODO to remove Flatten Maven Plugin
Fixed issues of concern include MSHADE-252, MSHADE-396, MSHADE-391.
TODO: MSHADE-36 is in Maven Shade 3.3.0 -> it should be possible to
include the dependency-reduced POM there directly. So maybe, we can rid
of Flatten Maven Plugin.
Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>