Skip to content

Commit 4b06cfc

Browse files
authored
SCANJLIB-263 Fix regression when host URL contains a path (#239)
Also improve error messages on HTTP errors.
1 parent 5b67c37 commit 4b06cfc

File tree

18 files changed

+98
-130
lines changed

18 files changed

+98
-130
lines changed

its/it-tests/src/test/java/com/sonar/scanner/lib/it/ProxyTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ public void simple_analysis_with_proxy_auth() throws Exception {
207207

208208
BuildResult buildResult = scanner.executeSimpleProject(project("js-sample"), ORCHESTRATOR.getServer().getUrl(), params, Map.of());
209209
assertThat(buildResult.getLastStatus()).isNotZero();
210-
assertThat(buildResult.getLogs()).contains("Failed to query server version: HTTP 407 Proxy Authentication Required.");
210+
assertThat(buildResult.getLogs()).containsPattern("Failed to query server version: GET http://(.*)/api/server/version failed with HTTP 407 Proxy Authentication Required.");
211211
assertThat(seenByProxy).isEmpty();
212212

213213
params.put("sonar.scanner.proxyUser", PROXY_USER);

lib/src/main/java/org/sonarsource/scanner/lib/ScannerEngineBootstrapper.java

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -275,28 +275,17 @@ private static String getServerVersion(ScannerHttpClient scannerHttpClient) {
275275
}
276276
return serverVersion;
277277
} catch (Exception e2) {
278-
var ex = new MessageException("Failed to query server version: " + formatMessage(e2), e2);
278+
var ex = new MessageException("Failed to query server version: " + e2.getMessage(), e2);
279279
if (!e2.equals(httpException)) {
280280
ex.addSuppressed(httpException);
281281
}
282282
throw ex;
283283
}
284284
} catch (Exception e) {
285-
throw new MessageException("Failed to query server version: " + formatMessage(e), e);
285+
throw new MessageException("Failed to query server version: " + e.getMessage(), e);
286286
}
287287
}
288288

289-
private static String formatMessage(Exception e) {
290-
if (e instanceof HttpException) {
291-
String message = "HTTP " + ((HttpException) e).getCode();
292-
if (StringUtils.isNotBlank(e.getMessage())) {
293-
message += " " + e.getMessage();
294-
}
295-
return message;
296-
}
297-
return e.getMessage();
298-
}
299-
300289
private void initBootstrapDefaultValues(ScannerEndpoint endpoint) {
301290
setBootstrapPropertyIfNotAlreadySet(ScannerProperties.HOST_URL, endpoint.getWebEndpoint());
302291
setBootstrapPropertyIfNotAlreadySet(ScannerProperties.API_BASE_URL, endpoint.getApiEndpoint());

lib/src/main/java/org/sonarsource/scanner/lib/internal/endpoint/SonarQubeServer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
*/
2020
package org.sonarsource.scanner.lib.internal.endpoint;
2121

22-
import java.net.URI;
22+
import org.apache.commons.lang3.StringUtils;
2323

2424
public class SonarQubeServer extends ScannerEndpoint {
2525

@@ -28,7 +28,7 @@ public SonarQubeServer(String webEndpoint) {
2828
}
2929

3030
private static String buildApiEndpoint(String webEndpoint) {
31-
return URI.create(webEndpoint).resolve("/api/v2").toString();
31+
return StringUtils.removeEnd(webEndpoint, "/") + "/api/v2";
3232
}
3333

3434
}

lib/src/main/java/org/sonarsource/scanner/lib/internal/facade/forked/ScannerEngineLauncherFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ private static ScannerEngineMetadata getScannerEngineMetadata(ScannerHttpClient
8080
try {
8181
String response = scannerHttpClient.callRestApi(API_PATH_ENGINE);
8282
return new Gson().fromJson(response, ScannerEngineMetadata.class);
83-
} catch (IOException e) {
84-
throw new IllegalStateException("Failed to get scanner-engine metadata", e);
83+
} catch (Exception e) {
84+
throw new IllegalStateException("Failed to get the scanner-engine metadata", e);
8585
}
8686
}
8787

lib/src/main/java/org/sonarsource/scanner/lib/internal/facade/inprocess/BootstrapIndexDownloader.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ Collection<JarEntry> getIndex() {
4242
index = conn.callWebApi("/batch/index");
4343
LOG.debug("Get bootstrap completed");
4444
} catch (Exception e) {
45-
throw new IllegalStateException("Fail to get bootstrap index from server", e);
45+
throw new IllegalStateException("Failed to get the bootstrap index from the server", e);
4646
}
4747
return parse(index);
4848
}
4949

50-
private Collection<JarEntry> parse(String index) {
50+
private static Collection<JarEntry> parse(String index) {
5151
final Collection<JarEntry> entries = new ArrayList<>();
5252

5353
String[] lines = index.split("[\r\n]+");
@@ -60,7 +60,7 @@ private Collection<JarEntry> parse(String index) {
6060
entries.add(new JarEntry(filename, hash));
6161
} catch (Exception e) {
6262
LOG.error("Failed bootstrap index response: {}", index);
63-
throw new IllegalStateException("Fail to parse entry in bootstrap index: " + line);
63+
throw new IllegalStateException("Failed to parse the entry in the bootstrap index: " + line);
6464
}
6565
}
6666

lib/src/main/java/org/sonarsource/scanner/lib/internal/http/HttpException.java

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,44 +20,31 @@
2020
package org.sonarsource.scanner.lib.internal.http;
2121

2222
import java.net.URL;
23-
import java.util.Objects;
2423
import javax.annotation.Nullable;
24+
import org.apache.commons.lang3.StringUtils;
25+
import org.slf4j.LoggerFactory;
2526

2627
public class HttpException extends RuntimeException {
27-
private final URL requestUrl;
2828
private final int code;
29-
@Nullable
30-
private final String body;
3129

32-
public HttpException(URL requestUrl, int code, String message, @Nullable String body) {
33-
super(message);
34-
this.requestUrl = requestUrl;
35-
this.code = code;
36-
this.body = body;
30+
public HttpException(URL requestUrl, int statusCode, String statusText, @Nullable String body) {
31+
super(formatMessage(requestUrl, statusCode, statusText, body));
32+
this.code = statusCode;
3733
}
3834

39-
public URL getRequestUrl() {
40-
return requestUrl;
35+
private static String formatMessage(URL requestUrl, int code, String statusText, @Nullable String body) {
36+
String message = "GET " + requestUrl + " failed with HTTP " + code;
37+
if (StringUtils.isNotBlank(statusText)) {
38+
message += " " + statusText;
39+
}
40+
if (LoggerFactory.getLogger(HttpException.class).isDebugEnabled() && StringUtils.isNotBlank(body)) {
41+
message += "\n" + body;
42+
}
43+
return message;
4144
}
4245

4346
public int getCode() {
4447
return code;
4548
}
4649

47-
@Nullable
48-
public String getBody() {
49-
return body;
50-
}
51-
52-
@Override
53-
public boolean equals(Object o) {
54-
if (!(o instanceof HttpException)) return false;
55-
HttpException that = (HttpException) o;
56-
return code == that.code && Objects.equals(requestUrl, that.requestUrl) && Objects.equals(body, that.body) && Objects.equals(getMessage(), that.getMessage());
57-
}
58-
59-
@Override
60-
public int hashCode() {
61-
return Objects.hash(requestUrl, code, body, getMessage());
62-
}
6350
}

lib/src/main/java/org/sonarsource/scanner/lib/internal/http/ScannerHttpClient.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ public void downloadFromExternalUrl(String url, Path toFile) {
7979
* @param url the URL of the file to download
8080
* @param toFile the target file
8181
* @param authentication if true, the request will be authenticated with the token
82-
* @throws IOException if connectivity problem or timeout (network) or IO error (when writing to file)
8382
* @throws IllegalStateException if HTTP response code is different than 2xx
8483
*/
8584
private void downloadFile(String url, Path toFile, boolean authentication) {
@@ -96,15 +95,15 @@ private void downloadFile(String url, Path toFile, boolean authentication) {
9695
});
9796
}
9897

99-
public String callRestApi(String urlPath) throws IOException {
98+
public String callRestApi(String urlPath) {
10099
if (!urlPath.startsWith("/")) {
101100
throw new IllegalArgumentException(format(EXCEPTION_MESSAGE_MISSING_SLASH, urlPath));
102101
}
103102
String url = httpConfig.getRestApiBaseUrl() + urlPath;
104103
return callApi(url);
105104
}
106105

107-
public String callWebApi(String urlPath) throws IOException {
106+
public String callWebApi(String urlPath) {
108107
if (!urlPath.startsWith("/")) {
109108
throw new IllegalArgumentException(format(EXCEPTION_MESSAGE_MISSING_SLASH, urlPath));
110109
}
@@ -116,7 +115,6 @@ public String callWebApi(String urlPath) throws IOException {
116115
* Call a server API and get the response as a string.
117116
*
118117
* @param url the url to call
119-
* @throws IOException if connectivity problem or timeout (network)
120118
* @throws IllegalStateException if HTTP response code is different than 2xx
121119
*/
122120
private String callApi(String url) {

lib/src/test/java/org/sonarsource/scanner/lib/ScannerEngineBootstrapperTest.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class ScannerEngineBootstrapperTest {
8282
private Path dumpFile;
8383

8484
@BeforeEach
85-
public void setUp() {
85+
void setUp() {
8686
this.dumpFile = dumpToFolder.resolve("dump.properties");
8787

8888
when(system.getProperty("os.name")).thenReturn("linux_ubuntu");
@@ -121,18 +121,19 @@ void should_use_new_bootstrapping_with_sonarqube_10_6() throws Exception {
121121

122122
@Test
123123
void should_report_apiv2_error_with_sq_10_6_even_if_older_version_ws_succeeded() throws Exception {
124-
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver").toURL(), 401, "", null));
124+
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver/api/v2/analysis/version").toURL(), 401, "", null));
125125
when(scannerHttpClient.callWebApi("/api/server/version")).thenReturn(SQ_VERSION_NEW_BOOTSTRAPPING);
126126
try (var bootstrapResult = underTest.setBootstrapProperty(ScannerProperties.HOST_URL, "http://localhost").bootstrap()) {
127127
assertThat(bootstrapResult.isSuccessful()).isFalse();
128128
}
129129

130-
assertThat(logTester.logs(Level.ERROR)).contains("Failed to query server version: HTTP 401. Please check the property sonar.token or the environment variable SONAR_TOKEN.");
130+
assertThat(logTester.logs(Level.ERROR))
131+
.contains("Failed to query server version: GET http://myserver/api/v2/analysis/version failed with HTTP 401. Please check the property sonar.token or the environment variable SONAR_TOKEN.");
131132
}
132133

133134
@Test
134135
void should_report_technical_errors() throws Exception {
135-
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new IOException("Socket closed"));
136+
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new IllegalStateException("Socket closed"));
136137
try (var bootstrapResult = underTest.setBootstrapProperty(ScannerProperties.HOST_URL, "http://localhost").bootstrap()) {
137138
assertThat(bootstrapResult.isSuccessful()).isFalse();
138139
}
@@ -220,16 +221,16 @@ void should_show_help_on_proxy_auth_error() throws Exception {
220221

221222
ScannerEngineBootstrapper bootstrapper = new ScannerEngineBootstrapper("Gradle", "3.1", system, scannerHttpClient,
222223
launcherFactory, scannerEngineLauncherFactory);
223-
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver").toURL(), 407, "Proxy Authentication Required", null));
224-
when(scannerHttpClient.callWebApi("/api/server/version")).thenThrow(new HttpException(URI.create("http://myserver").toURL(), 407, "Proxy Authentication Required", null));
224+
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver/analysis/version").toURL(), 407, "Proxy Authentication Required", null));
225+
when(scannerHttpClient.callWebApi("/api/server/version")).thenThrow(new HttpException(URI.create("http://myserver/api/server/version").toURL(), 407, "Proxy Authentication Required", null));
225226

226227
logTester.setLevel(Level.DEBUG);
227228

228229
try (var result = bootstrapper.setBootstrapProperty(ScannerProperties.HOST_URL, "http://myserver").bootstrap()) {
229230
assertThat(result.isSuccessful()).isFalse();
230231
}
231232

232-
assertThat(logTester.logs(Level.ERROR)).contains("Failed to query server version: HTTP 407 Proxy Authentication Required. Please check the properties sonar.scanner.proxyUser and " +
233+
assertThat(logTester.logs(Level.ERROR)).contains("Failed to query server version: GET http://myserver/api/server/version failed with HTTP 407 Proxy Authentication Required. Please check the properties sonar.scanner.proxyUser and " +
233234
"sonar.scanner.proxyPassword.");
234235
assertThatNoServerTypeIsLogged();
235236
}
@@ -242,8 +243,8 @@ void should_preserve_both_exceptions_when_checking_version() throws Exception {
242243

243244
ScannerEngineBootstrapper bootstrapper = new ScannerEngineBootstrapper("Gradle", "3.1", system, scannerHttpClient,
244245
launcherFactory, scannerEngineLauncherFactory);
245-
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver").toURL(), 404, "Not Found", null));
246-
when(scannerHttpClient.callWebApi("/api/server/version")).thenThrow(new HttpException(URI.create("http://myserver").toURL(), 400, "Server Error", null));
246+
when(scannerHttpClient.callRestApi("/analysis/version")).thenThrow(new HttpException(URI.create("http://myserver/analysis/version").toURL(), 404, "Not Found", null));
247+
when(scannerHttpClient.callWebApi("/api/server/version")).thenThrow(new HttpException(URI.create("http://myserver/api/server/version").toURL(), 400, "Server Error", null));
247248

248249
logTester.setLevel(Level.DEBUG);
249250

@@ -253,11 +254,11 @@ void should_preserve_both_exceptions_when_checking_version() throws Exception {
253254

254255
var loggedError = logTester.logEvents(Level.ERROR);
255256
assertThat(loggedError).hasSize(1);
256-
assertThat(loggedError.get(0).getFormattedMessage()).contains("Failed to query server version: HTTP 400 Server Error");
257+
assertThat(loggedError.get(0).getFormattedMessage()).contains("Failed to query server version: GET http://myserver/api/server/version failed with HTTP 400 Server Error");
257258
assertThat(ThrowableProxyUtil.asString(loggedError.get(0).getThrowableProxy()))
258259
.containsSubsequence(
259-
"Suppressed: org.sonarsource.scanner.lib.internal.http.HttpException: Not Found",
260-
"Caused by: org.sonarsource.scanner.lib.internal.http.HttpException: Server Error");
260+
"Suppressed: org.sonarsource.scanner.lib.internal.http.HttpException: GET http://myserver/analysis/version failed with HTTP 404 Not Found",
261+
"Caused by: org.sonarsource.scanner.lib.internal.http.HttpException: GET http://myserver/api/server/version failed with HTTP 400 Server Error");
261262
assertThatNoServerTypeIsLogged();
262263
}
263264

@@ -278,7 +279,7 @@ void should_log_user_friendly_message_when_auth_error(int code) throws Exception
278279
assertThat(result.isSuccessful()).isFalse();
279280
}
280281

281-
assertThat(logTester.logs(Level.ERROR)).contains("Failed to query server version: HTTP " + code + " Unauthorized. Please check the property sonar.token or the environment variable SONAR_TOKEN" +
282+
assertThat(logTester.logs(Level.ERROR)).contains("Failed to query server version: GET http://myserver failed with HTTP " + code + " Unauthorized. Please check the property sonar.token or the environment variable SONAR_TOKEN" +
282283
".");
283284
assertThatNoServerTypeIsLogged();
284285
}

lib/src/test/java/org/sonarsource/scanner/lib/internal/cache/FileCacheTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class FileCacheTest {
4444
private Path temp;
4545

4646
@BeforeEach
47-
public void setUp() {
47+
void setUp() {
4848
fileHashes = mock(FileHashes.class);
4949
cache = new FileCache(temp, fileHashes);
5050
}

lib/src/test/java/org/sonarsource/scanner/lib/internal/endpoint/ScannerEndpointResolverTest.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,17 @@ void should_recognize_sonarqube_cloud_endpoint_passed_through_host_url() {
4949
assertThat(endpoint.getApiEndpoint()).isEqualTo("https://api.sonarcloud.io");
5050
}
5151

52+
@Test
53+
void should_recognize_sonarqube_server_endpoint_with_path() {
54+
var props = Map.of(ScannerProperties.HOST_URL, "https://next.sonarqube.com/sonarqube");
55+
56+
var endpoint = ScannerEndpointResolver.resolveEndpoint(props);
57+
58+
assertThat(endpoint.isSonarQubeCloud()).isFalse();
59+
assertThat(endpoint.getWebEndpoint()).isEqualTo("https://next.sonarqube.com/sonarqube");
60+
assertThat(endpoint.getApiEndpoint()).isEqualTo("https://next.sonarqube.com/sonarqube/api/v2");
61+
}
62+
5263
@Test
5364
void should_recognize_sonarqube_cloud_endpoint_passed_through_cloud_url() {
5465
var props = Map.of(ScannerProperties.SONARQUBE_CLOUD_URL, "https://sonarcloud.io");

0 commit comments

Comments
 (0)