Skip to content

Commit 2697a3a

Browse files
authored
Patcher for AWS SDKv2 locale-dependent formatting (#126326)
AWS SDK v2 has a bug (aws/aws-sdk-java-v2#5968) where PathResolver uses locale-dependent formatting. This PR adds a patcher to the discovery-ec2 build process to replace calls to String.format(<format>, <args>) with String.format(Locale.ROOT, <format>, <args>). Relates to ES-11279
1 parent 07cb14e commit 2697a3a

File tree

4 files changed

+187
-2
lines changed

4 files changed

+187
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.gradle.internal.dependencies.patches.awsv2sdk;
11+
12+
import org.elasticsearch.gradle.internal.dependencies.patches.PatcherInfo;
13+
import org.elasticsearch.gradle.internal.dependencies.patches.Utils;
14+
import org.gradle.api.artifacts.transform.CacheableTransform;
15+
import org.gradle.api.artifacts.transform.InputArtifact;
16+
import org.gradle.api.artifacts.transform.TransformAction;
17+
import org.gradle.api.artifacts.transform.TransformOutputs;
18+
import org.gradle.api.artifacts.transform.TransformParameters;
19+
import org.gradle.api.file.FileSystemLocation;
20+
import org.gradle.api.provider.Provider;
21+
import org.gradle.api.tasks.Classpath;
22+
import org.jetbrains.annotations.NotNull;
23+
24+
import java.io.File;
25+
import java.util.List;
26+
27+
import static org.elasticsearch.gradle.internal.dependencies.patches.PatcherInfo.classPatcher;
28+
29+
@CacheableTransform
30+
public abstract class Awsv2ClassPatcher implements TransformAction<TransformParameters.None> {
31+
32+
private static final String JAR_FILE_TO_PATCH = "aws-query-protocol";
33+
34+
private static final List<PatcherInfo> CLASS_PATCHERS = List.of(
35+
// This patcher is needed because of this AWS bug: https://github.com/aws/aws-sdk-java-v2/issues/5968
36+
// As soon as the bug is resolved and we upgrade our AWS SDK v2 libraries, we can remove this.
37+
classPatcher(
38+
"software/amazon/awssdk/protocols/query/internal/marshall/ListQueryMarshaller.class",
39+
"213e84d9a745bdae4b844334d17aecdd6499b36df32aa73f82dc114b35043009",
40+
StringFormatInPathResolverPatcher::new
41+
)
42+
);
43+
44+
@Classpath
45+
@InputArtifact
46+
public abstract Provider<FileSystemLocation> getInputArtifact();
47+
48+
@Override
49+
public void transform(@NotNull TransformOutputs outputs) {
50+
File inputFile = getInputArtifact().get().getAsFile();
51+
52+
if (inputFile.getName().startsWith(JAR_FILE_TO_PATCH)) {
53+
System.out.println("Patching " + inputFile.getName());
54+
File outputFile = outputs.file(inputFile.getName().replace(".jar", "-patched.jar"));
55+
Utils.patchJar(inputFile, outputFile, CLASS_PATCHERS);
56+
} else {
57+
System.out.println("Skipping " + inputFile.getName());
58+
outputs.file(getInputArtifact());
59+
}
60+
}
61+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.gradle.internal.dependencies.patches.awsv2sdk;
11+
12+
import org.objectweb.asm.ClassVisitor;
13+
import org.objectweb.asm.ClassWriter;
14+
import org.objectweb.asm.MethodVisitor;
15+
import org.objectweb.asm.Type;
16+
17+
import java.util.Locale;
18+
19+
import static org.objectweb.asm.Opcodes.ASM9;
20+
import static org.objectweb.asm.Opcodes.GETSTATIC;
21+
import static org.objectweb.asm.Opcodes.INVOKESTATIC;
22+
23+
class StringFormatInPathResolverPatcher extends ClassVisitor {
24+
25+
StringFormatInPathResolverPatcher(ClassWriter classWriter) {
26+
super(ASM9, classWriter);
27+
}
28+
29+
@Override
30+
public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) {
31+
return new ReplaceCallMethodVisitor(super.visitMethod(access, name, descriptor, signature, exceptions));
32+
}
33+
34+
/**
35+
* Replaces calls to String.format(format, args); with calls to String.format(Locale.ROOT, format, args);
36+
*/
37+
private static class ReplaceCallMethodVisitor extends MethodVisitor {
38+
private static final String CLASS_INTERNAL_NAME = Type.getInternalName(String.class);
39+
private static final String METHOD_NAME = "format";
40+
private static final String OLD_METHOD_DESCRIPTOR = Type.getMethodDescriptor(
41+
Type.getType(String.class),
42+
Type.getType(String.class),
43+
Type.getType(Object[].class)
44+
);
45+
private static final String NEW_METHOD_DESCRIPTOR = Type.getMethodDescriptor(
46+
Type.getType(String.class),
47+
Type.getType(Locale.class),
48+
Type.getType(String.class),
49+
Type.getType(Object[].class)
50+
);
51+
52+
private boolean foundFormatPattern = false;
53+
54+
ReplaceCallMethodVisitor(MethodVisitor methodVisitor) {
55+
super(ASM9, methodVisitor);
56+
}
57+
58+
@Override
59+
public void visitLdcInsn(Object value) {
60+
if (value instanceof String s && s.startsWith("%s")) {
61+
if (foundFormatPattern) {
62+
throw new IllegalStateException(
63+
"A previous string format constant was not paired with a String.format() call. "
64+
+ "Patching would generate an unbalances stack"
65+
);
66+
}
67+
// Push the extra arg on the stack
68+
mv.visitFieldInsn(GETSTATIC, Type.getInternalName(Locale.class), "ROOT", Type.getDescriptor(Locale.class));
69+
foundFormatPattern = true;
70+
}
71+
super.visitLdcInsn(value);
72+
}
73+
74+
@Override
75+
public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
76+
if (opcode == INVOKESTATIC
77+
&& foundFormatPattern
78+
&& CLASS_INTERNAL_NAME.equals(owner)
79+
&& METHOD_NAME.equals(name)
80+
&& OLD_METHOD_DESCRIPTOR.equals(descriptor)) {
81+
// Replace the call with String.format(Locale.ROOT, format, args)
82+
mv.visitMethodInsn(INVOKESTATIC, CLASS_INTERNAL_NAME, METHOD_NAME, NEW_METHOD_DESCRIPTOR, false);
83+
foundFormatPattern = false;
84+
} else {
85+
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
86+
}
87+
}
88+
}
89+
}

plugins/discovery-ec2/build.gradle

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,31 @@ esplugin {
1515
classname ='org.elasticsearch.discovery.ec2.Ec2DiscoveryPlugin'
1616
}
1717

18+
def patched = Attribute.of('patched', Boolean)
19+
20+
configurations {
21+
compileClasspath {
22+
attributes {
23+
attribute(patched, true)
24+
}
25+
}
26+
runtimeClasspath {
27+
attributes {
28+
attribute(patched, true)
29+
}
30+
}
31+
testCompileClasspath {
32+
attributes {
33+
attribute(patched, true)
34+
}
35+
}
36+
testRuntimeClasspath {
37+
attributes {
38+
attribute(patched, true)
39+
}
40+
}
41+
}
42+
1843
dependencies {
1944

2045
implementation "software.amazon.awssdk:annotations:${versions.awsv2sdk}"
@@ -65,6 +90,17 @@ dependencies {
6590
testImplementation project(':test:fixtures:ec2-imds-fixture')
6691

6792
internalClusterTestImplementation project(':test:fixtures:ec2-imds-fixture')
93+
94+
attributesSchema {
95+
attribute(patched)
96+
}
97+
artifactTypes.getByName("jar") {
98+
attributes.attribute(patched, false)
99+
}
100+
registerTransform(org.elasticsearch.gradle.internal.dependencies.patches.awsv2sdk.Awsv2ClassPatcher) {
101+
from.attribute(patched, false)
102+
to.attribute(patched, true)
103+
}
68104
}
69105

70106
tasks.named("dependencyLicenses").configure {

plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/Ec2DiscoveryTests.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ protected List<TransportAddress> buildDynamicHosts(Settings nodeSettings, int no
103103
final String[] params = request.split("&");
104104
Arrays.stream(params).filter(entry -> entry.startsWith("Filter.") && entry.contains("=tag%3A")).forEach(entry -> {
105105
final int startIndex = "Filter.".length();
106-
// TODO ensure the filterId is an ASCII int when https://github.com/aws/aws-sdk-java-v2/issues/5968 fixed
107-
final var filterId = entry.substring(startIndex, entry.indexOf(".", startIndex));
106+
final int filterId = Integer.parseInt(entry.substring(startIndex, entry.indexOf(".", startIndex)));
108107
tagsIncluded.put(
109108
entry.substring(entry.indexOf("=tag%3A") + "=tag%3A".length()),
110109
Arrays.stream(params)

0 commit comments

Comments
 (0)