Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix windows encoding issues #2206

Merged
merged 3 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
-x spotlessCheck
-x checkstyleMain
-x checkstyleTest
-x spotbugsMain
peternied marked this conversation as resolved.
Show resolved Hide resolved

- name: Coverage
uses: codecov/codecov-action@v1
Expand Down Expand Up @@ -80,6 +81,7 @@ jobs:
-x spotlessCheck
-x checkstyleMain
-x checkstyleTest
-x spotbugsMain

backward-compatibility:
runs-on: ubuntu-latest
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/code-hygiene.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,18 @@ jobs:
- uses: gradle/gradle-build-action@v2
with:
arguments: checkstyleMain checkstyleTest

spotbugs:
runs-on: ubuntu-latest
name: Spotbugs scan
steps:
- uses: actions/checkout@v2

- uses: actions/setup-java@v2
with:
distribution: temurin # Temurin is a distribution of adoptium
java-version: 11

- uses: gradle/gradle-build-action@v2
with:
arguments: spotbugsMain
9 changes: 9 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ plugins {
id 'nebula.ospackage' version "8.3.0"
id "org.gradle.test-retry" version "1.3.1"
id 'eclipse'
id "com.github.spotbugs" version "5.0.13"
}

allprojects {
Expand Down Expand Up @@ -85,6 +86,14 @@ spotless {
}
}

spotbugs {
includeFilter = file('spotbugs-include.xml')
}

spotbugsTest {
enabled = false
}

java.sourceCompatibility = JavaVersion.VERSION_11
java.targetCompatibility = JavaVersion.VERSION_11

Expand Down
5 changes: 5 additions & 0 deletions spotbugs-include.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<FindBugsFilter>
<Match>
<Bug category="I18N" />
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.security.AccessController;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
Expand Down Expand Up @@ -152,7 +153,7 @@ private AuthTokenProcessorAction.Response handleImpl(RestRequest restRequest, Re
SettingsException {
if (token_log.isDebugEnabled()) {
try {
token_log.debug("SAMLResponse for {}\n{}", samlRequestId, new String(Util.base64decoder(samlResponseBase64), "UTF-8"));
token_log.debug("SAMLResponse for {}\n{}", samlRequestId, new String(Util.base64decoder(samlResponseBase64), StandardCharsets.UTF_8));
} catch (Exception e) {
token_log.warn(
"SAMLResponse for {} cannot be decoded from base64\n{}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, originalResult.internalSourceRef(), XContentType.JSON)) {
Object base64 = parser.map().values().iterator().next();
if(base64 instanceof String) {
originalSource = (new String(BaseEncoding.base64().decode((String) base64)));
originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
peternied marked this conversation as resolved.
Show resolved Hide resolved
} else {
originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
}
Expand All @@ -430,7 +430,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, currentIndex.source(), XContentType.JSON)) {
Object base64 = parser.map().values().iterator().next();
if(base64 instanceof String) {
currentSource = (new String(BaseEncoding.base64().decode((String) base64)));
currentSource = new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8);
} else {
currentSource = XContentHelper.convertToJson(currentIndex.source(), false, XContentType.JSON);
}
Expand All @@ -457,7 +457,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
try (XContentParser parser = XContentHelper.createParser(NamedXContentRegistry.EMPTY, THROW_UNSUPPORTED_OPERATION, currentIndex.source(), XContentType.JSON)) {
Object base64 = parser.map().values().iterator().next();
if(base64 instanceof String) {
msg.addSecurityConfigContentToRequestBody(new String(BaseEncoding.base64().decode((String) base64)), id);
msg.addSecurityConfigContentToRequestBody(new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8), id);
} else {
msg.addSecurityConfigTupleToRequestBody(new Tuple<XContentType, BytesReference>(XContentType.JSON, currentIndex.source()), id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
package org.opensearch.security.configuration;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -253,7 +254,7 @@ private SecurityDynamicConfiguration<?> toConfig(GetResponse singleGetResponse,

parser.nextToken();

final String jsonAsString = SecurityUtils.replaceEnvVars(new String(parser.binaryValue()), settings);
final String jsonAsString = SecurityUtils.replaceEnvVars(new String(parser.binaryValue(), StandardCharsets.UTF_8), settings);
final JsonNode jsonNode = DefaultObjectMapper.readTree(jsonAsString);
int configVersion = 1;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
Expand Down Expand Up @@ -646,7 +647,7 @@ private boolean areSameCerts(final X509Certificate[] currentX509Certs, final X50

final Function<? super X509Certificate, String> certificateSignature = cert -> {
final byte[] signature = cert !=null && cert.getSignature() != null ? cert.getSignature() : null;
return new String(signature);
return new String(signature, StandardCharsets.UTF_8);
};

final Set<String> currentCertSignatureSet = Arrays.stream(currentX509Certs)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.io.IOException;
import java.io.Reader;
import java.io.StringReader;
import java.nio.charset.StandardCharsets;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -91,7 +92,7 @@ public static void uploadFile(Client tc, String filepath, String index, CType cT
public static Reader createFileOrStringReader(CType cType, int configVersion, String filepath, boolean populateEmptyIfFileMissing) throws Exception {
Reader reader;
if (!populateEmptyIfFileMissing || new File(filepath).exists()) {
reader = new FileReader(filepath);
reader = new FileReader(filepath, StandardCharsets.UTF_8);
} else {
reader = new StringReader(createEmptySdcYaml(cType, configVersion));
}
Expand Down Expand Up @@ -143,7 +144,7 @@ public static <T> SecurityDynamicConfiguration<T> fromYamlReader(Reader yamlRead
}

public static <T> SecurityDynamicConfiguration<T> fromYamlFile(String filepath, CType ctype, int version, long seqNo, long primaryTerm) throws IOException {
return fromYamlReader(new FileReader(filepath), ctype, version, seqNo, primaryTerm);
return fromYamlReader(new FileReader(filepath, StandardCharsets.UTF_8), ctype, version, seqNo, primaryTerm);
}

public static <T> SecurityDynamicConfiguration<T> fromYamlString(String yamlString, CType ctype, int version, long seqNo, long primaryTerm) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ private static boolean retrieveFile(final RestHighLevelClient restHighLevelClien
}

System.out.println("Will retrieve '"+"/" +id+"' into "+filepath+" "+(legacy?"(legacy mode)":""));
try (Writer writer = new FileWriter(filepath)) {
try (Writer writer = new FileWriter(filepath, StandardCharsets.UTF_8)) {

final GetResponse response = restHighLevelClient.get(new GetRequest(index).id(id).refresh(true).realtime(false), RequestOptions.DEFAULT);

Expand Down
2 changes: 0 additions & 2 deletions src/test/java/org/opensearch/security/IntegrationTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Ignore;
import org.junit.Test;

import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest;
Expand Down Expand Up @@ -270,7 +269,6 @@ public void testSingle() throws Exception {
}

@Test
@Ignore // https://github.com/opensearch-project/security/issues/2194
public void testSpecialUsernames() throws Exception {

setup();
Expand Down