Skip to content

Commit fc238ad

Browse files
daniel-kmiecikdaniel-kmiecik
committed
fix: after review
1 parent dcc0216 commit fc238ad

File tree

5 files changed

+138
-68
lines changed

5 files changed

+138
-68
lines changed

‎modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/reference/ReferenceVisitor.java‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import io.swagger.v3.oas.models.security.SecurityScheme;
1616
import io.swagger.v3.parser.core.models.AuthorizationValue;
1717
import io.swagger.v3.parser.urlresolver.PermittedUrlsChecker;
18-
import io.swagger.v3.parser.urlresolver.exceptions.HostDeniedException;
1918
import io.swagger.v3.parser.util.RemoteUrl;
2019
import org.apache.commons.lang3.StringUtils;
2120
import org.slf4j.LoggerFactory;
@@ -86,7 +85,7 @@ public Reference toReference(String uri) throws Exception{
8685
return ref;
8786
}
8887

89-
public Reference toSchemaReference(String baseUri, JsonNode node) throws Exception{
88+
public Reference toSchemaReference(String baseUri, JsonNode node) {
9089
Map<String, Reference> referenceSet = this.reference.getReferenceSet();
9190
if (referenceSet.containsKey(baseUri)) {
9291
return referenceSet.get(baseUri);
@@ -199,17 +198,18 @@ public Header visitHeader(Header header){
199198
public String readHttp(String uri, List<AuthorizationValue> auths, PermittedUrlsChecker permittedUrlsChecker) throws Exception {
200199
if(context.getParseOptions().isSafelyResolveURL()){
201200
permittedUrlsChecker.verify(uri);
201+
return RemoteUrl.urlToString(uri, auths, permittedUrlsChecker);
202202
}
203-
return RemoteUrl.urlToString(uri, auths, permittedUrlsChecker);
203+
return RemoteUrl.urlToString(uri, auths);
204204
}
205205

206206
public<T> T resolveRef(T visiting, String ref, Class<T> clazz, BiFunction<T, ReferenceVisitor, T> traverseFunction){
207207
try {
208-
Reference reference = toReference(ref);
208+
Reference referenceObject = toReference(ref);
209209
String fragment = ReferenceUtils.getFragment(ref);
210-
JsonNode node = ReferenceUtils.jsonPointerEvaluate(fragment, reference.getJsonNode(), ref);
211-
T resolved = openAPITraverser.deserializeFragment(node, clazz, ref, fragment, reference.getMessages());
212-
ReferenceVisitor visitor = new ReferenceVisitor(reference, openAPITraverser, this.visited, this.visitedMap, context);
210+
JsonNode node = ReferenceUtils.jsonPointerEvaluate(fragment, referenceObject.getJsonNode(), ref);
211+
T resolved = openAPITraverser.deserializeFragment(node, clazz, ref, fragment, referenceObject.getMessages());
212+
ReferenceVisitor visitor = new ReferenceVisitor(referenceObject, openAPITraverser, this.visited, this.visitedMap, context);
213213
return traverseFunction.apply(resolved, visitor);
214214

215215
} catch (Exception e) {
@@ -229,39 +229,39 @@ public Schema resolveSchemaRef(Schema visiting, String ref, List<String> inherit
229229
}
230230
baseURI = ReferenceUtils.resolve(ref, baseURI);
231231
baseURI = ReferenceUtils.toBaseURI(baseURI);
232-
Reference reference = null;
232+
Reference referenceObject;
233233
boolean isAnchor = false;
234234
if (this.reference.getReferenceSet().containsKey(baseURI)) {
235-
reference = this.reference.getReferenceSet().get(baseURI);
235+
referenceObject = this.reference.getReferenceSet().get(baseURI);
236236
}
237237
else {
238-
JsonNode node = null;
238+
JsonNode node;
239239
try {
240240
node = parse(baseURI, this.reference.getAuths());
241241
} catch (Exception e) {
242242
// we can not parse, try ref
243243
baseURI = toBaseURI(ref);
244244
node = parse(baseURI, this.reference.getAuths());
245245
}
246-
reference = toSchemaReference(baseURI, node);
246+
referenceObject = toSchemaReference(baseURI, node);
247247
}
248248
String fragment = ReferenceUtils.getFragment(ref);
249-
JsonNode evaluatedNode = null;
249+
JsonNode evaluatedNode;
250250
try {
251-
evaluatedNode = ReferenceUtils.jsonPointerEvaluate(fragment, reference.getJsonNode(), ref);
251+
evaluatedNode = ReferenceUtils.jsonPointerEvaluate(fragment, referenceObject.getJsonNode(), ref);
252252
} catch (RuntimeException e) {
253253
// maybe anchor
254-
evaluatedNode = findAnchor(reference.getJsonNode(), fragment);
254+
evaluatedNode = findAnchor(referenceObject.getJsonNode(), fragment);
255255
if (evaluatedNode == null) {
256256
throw new RuntimeException("Could not find " + fragment + " in contents of " + ref);
257257
}
258258
isAnchor = true;
259259
}
260-
Schema resolved = openAPITraverser.deserializeFragment(evaluatedNode, Schema.class, ref, fragment, reference.getMessages());
260+
Schema resolved = openAPITraverser.deserializeFragment(evaluatedNode, Schema.class, ref, fragment, referenceObject.getMessages());
261261
if (isAnchor) {
262262
resolved.$anchor(null);
263263
}
264-
ReferenceVisitor visitor = new ReferenceVisitor(reference, openAPITraverser, this.visited, this.visitedMap, context);
264+
ReferenceVisitor visitor = new ReferenceVisitor(referenceObject, openAPITraverser, this.visited, this.visitedMap, context);
265265
return openAPITraverser.traverseSchema(resolved, visitor, inheritedIds);
266266
} catch (Exception e) {
267267
LOGGER.error("Error resolving schema " + ref, e);

‎modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/RemoteUrl.java‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,9 @@ public void checkServerTrusted(X509Certificate[] certs, String authType) {
8484
LOGGER.error("Not Supported", e);
8585
}
8686
}
87-
return new ConnectionConfigurator() {
88-
89-
@Override
90-
public void process(URLConnection connection) {
91-
connection.setConnectTimeout(CONNECTION_TIMEOUT);
92-
connection.setReadTimeout(READ_TIMEOUT);
93-
}
87+
return connection -> {
88+
connection.setConnectTimeout(CONNECTION_TIMEOUT);
89+
connection.setReadTimeout(READ_TIMEOUT);
9490
};
9591
}
9692

@@ -121,17 +117,7 @@ public static String urlToString(String url, List<AuthorizationValue> auths, Per
121117
final URL inUrl = new URL(cleanUrl(url));
122118
final List<AuthorizationValue> query = new ArrayList<>();
123119
final List<AuthorizationValue> header = new ArrayList<>();
124-
if (auths != null && !auths.isEmpty()) {
125-
for (AuthorizationValue auth : auths) {
126-
if (auth.getUrlMatcher() != null && auth.getUrlMatcher().test(inUrl)) {
127-
if ("query".equals(auth.getType())) {
128-
appendValue(inUrl, auth, query);
129-
} else if ("header".equals(auth.getType())) {
130-
appendValue(inUrl, auth, header);
131-
}
132-
}
133-
}
134-
}
120+
filterAndAssignAuthValues(auths, inUrl, query, header);
135121
conn = prepareConnection(query, inUrl);
136122
CONNECTION_CONFIGURATOR.process(conn);
137123
setRequestHeaders(header, conn);
@@ -161,6 +147,20 @@ public static String urlToString(String url, List<AuthorizationValue> auths, Per
161147
}
162148
}
163149

150+
private static void filterAndAssignAuthValues(List<AuthorizationValue> auths, URL inUrl, List<AuthorizationValue> query, List<AuthorizationValue> header) {
151+
if (auths != null && !auths.isEmpty()) {
152+
for (AuthorizationValue auth : auths) {
153+
if (auth.getUrlMatcher() != null && auth.getUrlMatcher().test(inUrl)) {
154+
if ("query".equals(auth.getType())) {
155+
appendValue(inUrl, auth, query);
156+
} else if ("header".equals(auth.getType())) {
157+
appendValue(inUrl, auth, header);
158+
}
159+
}
160+
}
161+
}
162+
}
163+
164164
private static String readResponse(URLConnection conn) throws IOException {
165165

166166
try (InputStream in = conn.getInputStream();
@@ -209,15 +209,13 @@ private static URLConnection prepareConnection(List<AuthorizationValue> query, U
209209

210210
private static boolean isRedirect(HttpURLConnection conn) throws IOException {
211211
int code = conn.getResponseCode();
212-
return code == 301 || code == 302 || code == 307 || code == 308;
212+
return code == 301 || code == 302 || code == 303 || code == 307 || code == 308;
213213
}
214214

215215
private static void appendValue(URL url, AuthorizationValue value, Collection<AuthorizationValue> to) {
216-
if (value instanceof ManagedValue) {
217-
if (!((ManagedValue) value).process(url)) {
216+
if (value instanceof ManagedValue && !((ManagedValue) value).process(url)) {
218217
return;
219218
}
220-
}
221219
to.add(value);
222220
}
223221

‎modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/PermittedUrlsCheckerAllowLocal.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package io.swagger.v3.parser.util;
2+
3+
import io.swagger.v3.parser.urlresolver.PermittedUrlsChecker;
4+
5+
import java.net.InetAddress;
6+
7+
class PermittedUrlsCheckerAllowLocal extends PermittedUrlsChecker {
8+
@Override
9+
protected boolean isRestrictedIpRange(InetAddress ip) {
10+
// Allow all IPs for testing purposes
11+
return false;
12+
}
13+
}

‎modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/util/RemoteUrlTest.java‎

Lines changed: 86 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,19 @@
22

33
import com.github.tomakehurst.wiremock.WireMockServer;
44
import com.github.tomakehurst.wiremock.client.WireMock;
5+
import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
56
import com.github.tomakehurst.wiremock.verification.LoggedRequest;
67
import io.swagger.v3.parser.core.models.AuthorizationValue;
8+
import io.swagger.v3.parser.urlresolver.exceptions.HostDeniedException;
79
import org.testng.annotations.AfterMethod;
810
import org.testng.annotations.BeforeMethod;
911
import org.testng.annotations.Test;
1012

1113
import javax.net.ssl.HttpsURLConnection;
14+
import java.io.IOException;
1215
import java.net.SocketTimeoutException;
1316
import java.net.URL;
14-
import java.util.Arrays;
17+
import java.util.Collections;
1518
import java.util.List;
1619

1720
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
@@ -35,15 +38,21 @@ public class RemoteUrlTest {
3538

3639

3740
@AfterMethod
38-
public void tearDown() throws Exception {
41+
public void tearDown() {
3942
wireMockServer.stop();
4043
}
4144

4245
@BeforeMethod
43-
public void setUp() throws Exception {
44-
wireMockServer = new WireMockServer(WIRE_MOCK_PORT);
46+
public void setUp() {
47+
System.setProperty("io.swagger.v3.parser.util.RemoteUrl.trustAll", "true");
48+
wireMockServer = new WireMockServer(
49+
WireMockConfiguration.options()
50+
.port(0) // disables HTTP
51+
.httpsPort(WIRE_MOCK_PORT) // enables HTTPS
52+
);
4553
wireMockServer.start();
46-
WireMock.configureFor(WIRE_MOCK_PORT);
54+
WireMock.configureFor("https", LOCALHOST, WIRE_MOCK_PORT); // Use HTTPS for WireMock
55+
4756
}
4857

4958
@Test
@@ -62,7 +71,7 @@ public void testReadARemoteUrl() throws Exception {
6271
assertEquals(actualBody, expectedBody);
6372

6473
verify(getRequestedFor(urlEqualTo("/v2/pet/1"))
65-
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER)));
74+
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER)));
6675
}
6776

6877
@Test
@@ -73,13 +82,13 @@ public void testAuthorizationHeader() throws Exception {
7382
final String headerName = "Authorization";
7483
final String headerValue = "foobar";
7584
final AuthorizationValue authorizationValue = new AuthorizationValue(headerName, headerValue, "header");
76-
final String actualBody = RemoteUrl.urlToString(getUrl(), Arrays.asList(authorizationValue));
85+
final String actualBody = RemoteUrl.urlToString(getUrl(), Collections.singletonList(authorizationValue));
7786

7887
assertEquals(actualBody, expectedBody);
7988

8089
verify(getRequestedFor(urlEqualTo("/v2/pet/1"))
81-
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER))
82-
.withHeader(headerName, equalTo(headerValue))
90+
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER))
91+
.withHeader(headerName, equalTo(headerValue))
8392
);
8493
}
8594

@@ -91,14 +100,14 @@ public void testAuthorizationHeaderWithMatchingUrl() throws Exception {
91100
final String headerName = "Authorization";
92101
final String headerValue = "foobar";
93102
final AuthorizationValue authorizationValue = new AuthorizationValue(headerName, headerValue, "header",
94-
url -> url.toString().startsWith("http://localhost"));
95-
final String actualBody = RemoteUrl.urlToString(getUrl(), Arrays.asList(authorizationValue));
103+
url -> url.toString().startsWith("https://localhost"));
104+
final String actualBody = RemoteUrl.urlToString(getUrl(), Collections.singletonList(authorizationValue));
96105

97106
assertEquals(actualBody, expectedBody);
98107

99108
verify(getRequestedFor(urlEqualTo("/v2/pet/1"))
100-
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER))
101-
.withHeader(headerName, equalTo(headerValue))
109+
.withHeader("Accept", equalTo(EXPECTED_ACCEPTS_HEADER))
110+
.withHeader(headerName, equalTo(headerValue))
102111
);
103112
}
104113

@@ -110,33 +119,32 @@ public void testAuthorizationHeaderWithNonMatchingUrl() throws Exception {
110119
final String headerValue = "foobar";
111120
String authorization = "Authorization";
112121
final AuthorizationValue authorizationValue = new AuthorizationValue(authorization,
113-
headerValue, "header", u -> false);
114-
final String actualBody = RemoteUrl.urlToString(getUrl(), Arrays.asList(authorizationValue));
122+
headerValue, "header", u -> false);
123+
final String actualBody = RemoteUrl.urlToString(getUrl(), Collections.singletonList(authorizationValue));
115124

116125
assertEquals(actualBody, expectedBody);
117126

118127
List<LoggedRequest> requests = WireMock.findAll(getRequestedFor(urlEqualTo("/v2/pet/1")));
119-
assertEquals(1, requests.size());
128+
assertEquals(requests.size(), 1);
120129
assertFalse(requests.get(0).containsHeader(authorization));
121130
}
122131

123132
private String getUrl() {
124-
return String.format("http://%s:%d/v2/pet/1", LOCALHOST, WIRE_MOCK_PORT);
133+
return String.format("https://%s:%d/v2/pet/1", LOCALHOST, WIRE_MOCK_PORT);
125134
}
126135

127136
private String setupStub() {
128137
final String expectedBody = "a really good body";
129138
stubFor(get(urlEqualTo("/v2/pet/1"))
130139
.willReturn(aResponse()
131-
.withBody(expectedBody)
132-
.withHeader("Content-Type", "application/json")
140+
.withBody(expectedBody)
141+
.withHeader("Content-Type", "application/json")
133142
));
134143
return expectedBody;
135144
}
136145

137146
@Test
138147
public void testConnectionTimeoutEnforced() throws Exception {
139-
System.setProperty("io.swagger.v3.parser.util.RemoteUrl.trustAll", "true");
140148
RemoteUrl.ConnectionConfigurator configurator = RemoteUrl.createConnectionConfigurator();
141149
URL url = new URL("https://10.255.255.1"); // non-routable IP to simulate timeout
142150
HttpsURLConnection conn = (HttpsURLConnection) url.openConnection();
@@ -152,4 +160,62 @@ public void testConnectionTimeoutEnforced() throws Exception {
152160
assertTrue(duration <= CONNECTION_TIMEOUT + 2000, "Timeout was not enforced properly (took too long)");
153161
}
154162
}
163+
164+
@Test(expectedExceptions = IOException.class)
165+
public void testTooManyRedirectsThrowsException() throws Exception {
166+
String nextPath = "/redirect";
167+
// Chain 6 redirects
168+
for (int i = 0; i < 6; i++) {
169+
String target = "/redirect" + (i + 1);
170+
stubFor(get(urlEqualTo(nextPath))
171+
.willReturn(aResponse()
172+
.withStatus(302)
173+
.withHeader("Location", wireMockServer.baseUrl() + target)));
174+
nextPath = target;
175+
}
176+
177+
// Add stub for /redirect6 to return a 200 OK response, but it's not expected to be reached
178+
stubFor(get(urlEqualTo(nextPath))
179+
.willReturn(aResponse()
180+
.withStatus(200)
181+
.withBody("Final destination")));
182+
183+
String startUrl = String.format("https://%s:%d/redirect", LOCALHOST, WIRE_MOCK_PORT);
184+
185+
try {
186+
RemoteUrl.urlToString(startUrl, null, new PermittedUrlsCheckerAllowLocal());
187+
} catch (IOException e) {
188+
assertTrue(e.getMessage().contains("Too many redirects"));
189+
throw e;
190+
}
191+
}
192+
193+
@Test(expectedExceptions = HostDeniedException.class)
194+
public void testRedirectWithForbiddenProtocolThrowsException() throws Exception {
195+
String nextPath = "/redirect";
196+
// Chain 6 redirects
197+
for (int i = 0; i < 3; i++) {
198+
String target = "/redirect" + (i + 1);
199+
stubFor(get(urlEqualTo(nextPath))
200+
.willReturn(aResponse()
201+
.withStatus(302)
202+
.withHeader("Location", "ftp://localhost/" + target)));
203+
nextPath = target;
204+
}
205+
206+
// Add stub for /redirect6 to return a 200 OK response, but it's not expected to be reached
207+
stubFor(get(urlEqualTo(nextPath))
208+
.willReturn(aResponse()
209+
.withStatus(200)
210+
.withBody("Final destination")));
211+
212+
String startUrl = String.format("https://%s:%d/redirect", LOCALHOST, WIRE_MOCK_PORT);
213+
214+
try {
215+
RemoteUrl.urlToString(startUrl, null, new PermittedUrlsCheckerAllowLocal());
216+
} catch (IOException e) {
217+
assertTrue(e.getMessage().contains("Too many redirects"));
218+
throw e;
219+
}
220+
}
155221
}

‎pom.xml‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,13 @@
5555
<artifactId>maven-surefire-plugin</artifactId>
5656
<version>${surefire-version}</version>
5757
<dependencies>
58-
<!--dependency>
59-
<groupId>org.apache.maven.surefire</groupId>
60-
<artifactId>surefire-junit4</artifactId>
61-
<version>3.0.0</version>
62-
</dependency>
63-
<dependency>
64-
<groupId>org.apache.maven.surefire</groupId>
65-
<artifactId>surefire-testng</artifactId>
66-
<version>3.1.2</version>
67-
</dependency-->
6858
</dependencies>
6959
<configuration>
7060
<testNGArtifactName>none:none</testNGArtifactName>
7161
<argLine>-javaagent:"${settings.localRepository}"/org/jmockit/jmockit/${jmockit-version}/jmockit-${jmockit-version}.jar --add-opens java.base/java.lang=ALL-UNNAMED -Djdk.attach.allowAttachSelf</argLine>
62+
<systemPropertyVariables>
63+
<io.swagger.v3.parser.util.RemoteUrl.trustAll>true</io.swagger.v3.parser.util.RemoteUrl.trustAll>
64+
</systemPropertyVariables>
7265
</configuration>
7366
<executions>
7467
<execution>

0 commit comments

Comments
 (0)