Changeset 18437 in josm
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/tools/HttpClient.java
r18409 r18437 189 189 " Can''t redirect. Aborting.", cr.getResponseCode())); 190 190 } else if (maxRedirects > 0) { 191 final URL oldUrl = url; 191 192 url = new URL(url, redirectLocation); 192 193 maxRedirects--; 193 194 logRequest(tr("Download redirected to ''{0}''", redirectLocation)); 195 // Fix JOSM #21935: Avoid leaking `Authorization` header on redirects. 196 if (!Objects.equals(oldUrl.getHost(), this.url.getHost()) && this.getRequestHeader("Authorization") != null) { 197 logRequest(tr("Download redirected to different host (''{0}'' -> ''{1}''), removing authorization headers", 198 oldUrl.getHost(), url.getHost())); 199 this.headers.remove("Authorization"); 200 } 194 201 response = connect(); 195 202 successfulConnection = true; -
trunk/test/functional/org/openstreetmap/josm/tools/HttpClientTest.java
r18106 r18437 16 16 import static org.hamcrest.MatcherAssert.assertThat; 17 17 import static org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase; 18 import static org.junit.jupiter.api.Assertions.assertAll; 18 19 import static org.junit.jupiter.api.Assertions.assertEquals; 20 import static org.junit.jupiter.api.Assertions.assertFalse; 19 21 import static org.junit.jupiter.api.Assertions.assertThrows; 20 22 import static org.junit.jupiter.api.Assertions.assertTrue; … … 30 32 import java.util.Collections; 31 33 import java.util.Map; 34 import java.util.UUID; 32 35 import java.util.logging.Handler; 33 36 import java.util.logging.LogRecord; … … 38 41 import org.junit.jupiter.api.Test; 39 42 import org.junit.jupiter.api.Timeout; 43 import org.junit.jupiter.params.ParameterizedTest; 44 import org.junit.jupiter.params.provider.ValueSource; 40 45 import org.openstreetmap.josm.TestUtils; 41 46 import org.openstreetmap.josm.data.Version; … … 47 52 48 53 import com.github.tomakehurst.wiremock.WireMockServer; 54 import com.github.tomakehurst.wiremock.admin.model.ServeEventQuery; 49 55 import com.github.tomakehurst.wiremock.http.HttpHeader; 50 56 import com.github.tomakehurst.wiremock.http.HttpHeaders; 51 57 import com.github.tomakehurst.wiremock.matching.UrlPattern; 58 import com.github.tomakehurst.wiremock.stubbing.ServeEvent; 52 59 53 60 /** … … 234 241 mockRedirects(false, 3); 235 242 assertThrows(IOException.class, () -> HttpClient.create(url("/relative-redirect/3")).setMaxRedirects(2).connect(progress)); 243 } 244 245 /** 246 * Ensure that we don't leak authorization headers 247 * See <a href="https://josm.openstreetmap.de/ticket/21935">JOSM #21935</a> 248 * @param authorization The various authorization configurations to test 249 */ 250 @ParameterizedTest 251 @ValueSource(strings = { "Basic dXNlcm5hbWU6cGFzc3dvcmQ=", "Digest username=test_user", 252 /* OAuth 1.0 for OSM as implemented in JOSM core */ 253 "OAuth oauth_consumer_key=\"test_key\", oauth_nonce=\"1234\", oauth_signature=\"test_signature\", " 254 + "oauth_signature_method=\"HMAC-SHA1\", oauth_timestamp=\"0\", oauth_token=\"test_token\", " 255 + "oauth_version=\"1.0\"", 256 /* OAuth 2.0, not yet implemented in JOSM core */ 257 "Bearer some_random_token" 258 }) 259 void testRedirectsToDifferentSite(String authorization) throws IOException { 260 final String localhost = "localhost"; 261 final String localhostIp = "127.0.0.1"; 262 final String otherServer = this.localServer.baseUrl().contains(localhost) ? localhostIp : localhost; 263 final UUID redirect = this.localServer.stubFor(get(urlEqualTo("/redirect/other-site")) 264 .willReturn(aResponse().withStatus(302).withHeader( 265 "Location", localServer.url("/same-site/other-site")))).getId(); 266 final UUID sameSite = this.localServer.stubFor(get(urlEqualTo("/same-site/other-site")) 267 .willReturn(aResponse().withStatus(302).withHeader( 268 "Location", localServer.url("/other-site") 269 .replace(otherServer == localhost ? localhostIp : localhost, otherServer)))).getId(); 270 final UUID otherSite = this.localServer.stubFor(get(urlEqualTo("/other-site")) 271 .willReturn(aResponse().withStatus(200).withBody("other-site-here"))).getId(); 272 final HttpClient client = HttpClient.create(url("/redirect/other-site")); 273 client.setHeader("Authorization", authorization); 274 try { 275 client.connect(); 276 this.localServer.getServeEvents(); 277 final ServeEvent first = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(redirect)).getRequests().get(0); 278 final ServeEvent second = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(sameSite)).getRequests().get(0); 279 final ServeEvent third = this.localServer.getServeEvents(ServeEventQuery.forStubMapping(otherSite)).getRequests().get(0); 280 assertAll(() -> assertEquals(3, this.localServer.getServeEvents().getRequests().size()), 281 () -> assertEquals(authorization, first.getRequest().getHeader("Authorization"), 282 "Authorization is expected for the first request: " + first.getRequest().getUrl()), 283 () -> assertEquals(authorization, second.getRequest().getHeader("Authorization"), 284 "Authorization is expected for the second request: " + second.getRequest().getUrl()), 285 () -> assertFalse(third.getRequest().containsHeader("Authorization"), 286 "Authorization is not expected for the third request: " + third.getRequest().getUrl())); 287 } finally { 288 client.disconnect(); 289 } 236 290 } 237 291
Note:
See TracChangeset
for help on using the changeset viewer.