--- a/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Fri Aug 30 15:42:27 2019 +0100
@@ -81,6 +81,8 @@
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.Properties;
+import java.util.concurrent.locks.ReentrantLock;
+
import static sun.net.www.protocol.http.AuthScheme.BASIC;
import static sun.net.www.protocol.http.AuthScheme.DIGEST;
import static sun.net.www.protocol.http.AuthScheme.NTLM;
@@ -97,7 +99,7 @@
public class HttpURLConnection extends java.net.HttpURLConnection {
- static String HTTP_CONNECT = "CONNECT";
+ static final String HTTP_CONNECT = "CONNECT";
static final String version;
public static final String userAgent;
@@ -352,7 +354,7 @@
* - getOutputStream()
* - getInputStream())
* - connect()
- * Access synchronized on this.
+ * Access synchronized on connectionLock.
*/
private boolean connecting = false;
@@ -384,6 +386,9 @@
/* Progress source */
protected ProgressSource pi;
+ /* Lock */
+ final ReentrantLock connectionLock = new ReentrantLock();
+
/* all the response headers we get back */
private MessageHeader responses;
/* the stream _from_ the server */
@@ -513,13 +518,18 @@
}
@Override
- public synchronized void setAuthenticator(Authenticator auth) {
- if (connecting || connected) {
- throw new IllegalStateException(
- "Authenticator must be set before connecting");
+ public void setAuthenticator(Authenticator auth) {
+ connectionLock.lock();
+ try {
+ if (connecting || connected) {
+ throw new IllegalStateException(
+ "Authenticator must be set before connecting");
+ }
+ authenticator = Objects.requireNonNull(auth);
+ authenticatorKey = AuthenticatorKeys.getKey(authenticator);
+ } finally {
+ connectionLock.unlock();
}
- authenticator = Objects.requireNonNull(auth);
- authenticatorKey = AuthenticatorKeys.getKey(authenticator);
}
public String getAuthenticatorKey() {
@@ -562,12 +572,17 @@
}
}
- public synchronized void setRequestMethod(String method)
+ public void setRequestMethod(String method)
throws ProtocolException {
- if (connecting) {
- throw new IllegalStateException("connect in progress");
+ connectionLock.lock();
+ try {
+ if (connecting) {
+ throw new IllegalStateException("connect in progress");
+ }
+ super.setRequestMethod(method);
+ } finally {
+ connectionLock.unlock();
}
- super.setRequestMethod(method);
}
/* adds the standard key/val pairs to reqests if necessary & write to
@@ -682,6 +697,8 @@
}
} else if (poster != null) {
/* add Content-Length & POST/PUT data */
+ // safe to synchronize on poster: this is
+ // a simple subclass of ByteArrayOutputStream
synchronized (poster) {
/* close it, so no more data can be added */
poster.close();
@@ -1009,8 +1026,11 @@
// overridden in HTTPS subclass
public void connect() throws IOException {
- synchronized (this) {
+ connectionLock.lock();
+ try {
connecting = true;
+ } finally {
+ connectionLock.unlock();
}
plainConnect();
}
@@ -1057,10 +1077,13 @@
}
protected void plainConnect() throws IOException {
- synchronized (this) {
+ connectionLock.lock();
+ try {
if (connected) {
return;
}
+ } finally {
+ connectionLock.unlock();
}
SocketPermission p = URLtoSocketPermission(this.url);
if (p != null) {
@@ -1323,28 +1346,34 @@
*/
@Override
- public synchronized OutputStream getOutputStream() throws IOException {
- connecting = true;
- SocketPermission p = URLtoSocketPermission(this.url);
-
- if (p != null) {
- try {
- return AccessController.doPrivilegedWithCombiner(
- new PrivilegedExceptionAction<>() {
- public OutputStream run() throws IOException {
- return getOutputStream0();
- }
- }, null, p
- );
- } catch (PrivilegedActionException e) {
- throw (IOException) e.getException();
+ public OutputStream getOutputStream() throws IOException {
+ connectionLock.lock();
+ try {
+ connecting = true;
+ SocketPermission p = URLtoSocketPermission(this.url);
+
+ if (p != null) {
+ try {
+ return AccessController.doPrivilegedWithCombiner(
+ new PrivilegedExceptionAction<>() {
+ public OutputStream run() throws IOException {
+ return getOutputStream0();
+ }
+ }, null, p
+ );
+ } catch (PrivilegedActionException e) {
+ throw (IOException) e.getException();
+ }
+ } else {
+ return getOutputStream0();
}
- } else {
- return getOutputStream0();
+ } finally {
+ connectionLock.unlock();
}
}
- private synchronized OutputStream getOutputStream0() throws IOException {
+ private OutputStream getOutputStream0() throws IOException {
+ assert connectionLock.isHeldByCurrentThread();
try {
if (!doOutput) {
throw new ProtocolException("cannot write to a URLConnection"
@@ -1434,17 +1463,19 @@
// we only want to capture the user defined Cookies once, as
// they cannot be changed by user code after we are connected,
// only internally.
- synchronized (this) {
- if (setUserCookies) {
- int k = requests.getKey("Cookie");
- if (k != -1)
- userCookies = requests.getValue(k);
- k = requests.getKey("Cookie2");
- if (k != -1)
- userCookies2 = requests.getValue(k);
- setUserCookies = false;
- }
- }
+ if (setUserCookies) {
+ // we should only reach here when called from
+ // writeRequest, which in turn is only called by
+ // getInputStream0
+ assert connectionLock.isHeldByCurrentThread();
+ int k = requests.getKey("Cookie");
+ if (k != -1)
+ userCookies = requests.getValue(k);
+ k = requests.getKey("Cookie2");
+ if (k != -1)
+ userCookies2 = requests.getValue(k);
+ setUserCookies = false;
+ }
// remove old Cookie header before setting new one.
requests.remove("Cookie");
@@ -1501,30 +1532,36 @@
}
@Override
- public synchronized InputStream getInputStream() throws IOException {
- connecting = true;
- SocketPermission p = URLtoSocketPermission(this.url);
-
- if (p != null) {
- try {
- return AccessController.doPrivilegedWithCombiner(
- new PrivilegedExceptionAction<>() {
- public InputStream run() throws IOException {
- return getInputStream0();
- }
- }, null, p
- );
- } catch (PrivilegedActionException e) {
- throw (IOException) e.getException();
+ public InputStream getInputStream() throws IOException {
+ connectionLock.lock();
+ try {
+ connecting = true;
+ SocketPermission p = URLtoSocketPermission(this.url);
+
+ if (p != null) {
+ try {
+ return AccessController.doPrivilegedWithCombiner(
+ new PrivilegedExceptionAction<>() {
+ public InputStream run() throws IOException {
+ return getInputStream0();
+ }
+ }, null, p
+ );
+ } catch (PrivilegedActionException e) {
+ throw (IOException) e.getException();
+ }
+ } else {
+ return getInputStream0();
}
- } else {
- return getInputStream0();
+ } finally {
+ connectionLock.unlock();
}
}
@SuppressWarnings("empty-statement")
- private synchronized InputStream getInputStream0() throws IOException {
-
+ private InputStream getInputStream0() throws IOException {
+
+ assert connectionLock.isHeldByCurrentThread();
if (!doInput) {
throw new ProtocolException("Cannot read from URLConnection"
+ " if doInput=false (call setDoInput(true))");
@@ -2053,7 +2090,16 @@
/**
* establish a tunnel through proxy server
*/
- public synchronized void doTunneling() throws IOException {
+ public void doTunneling() throws IOException {
+ connectionLock.lock();
+ try {
+ doTunneling0();
+ } finally{
+ connectionLock.unlock();
+ }
+ }
+
+ private void doTunneling0() throws IOException {
int retryTunnel = 0;
String statusLine = "";
int respCode = 0;
@@ -2423,7 +2469,7 @@
/**
* Gets the authentication for an HTTP server, and applies it to
* the connection.
- * @param authHdr the AuthenticationHeader which tells what auth scheme is
+ * @param authhdr the AuthenticationHeader which tells what auth scheme is
* preferred.
*/
@SuppressWarnings("fallthrough")
@@ -3166,17 +3212,22 @@
* @param value the value to be set
*/
@Override
- public synchronized void setRequestProperty(String key, String value) {
- if (connected || connecting)
- throw new IllegalStateException("Already connected");
- if (key == null)
- throw new NullPointerException ("key is null");
-
- if (isExternalMessageHeaderAllowed(key, value)) {
- requests.set(key, value);
- if (!key.equalsIgnoreCase("Content-Type")) {
- userHeaders.set(key, value);
+ public void setRequestProperty(String key, String value) {
+ connectionLock.lock();
+ try {
+ if (connected || connecting)
+ throw new IllegalStateException("Already connected");
+ if (key == null)
+ throw new NullPointerException("key is null");
+
+ if (isExternalMessageHeaderAllowed(key, value)) {
+ requests.set(key, value);
+ if (!key.equalsIgnoreCase("Content-Type")) {
+ userHeaders.set(key, value);
+ }
}
+ } finally {
+ connectionLock.unlock();
}
}
@@ -3192,21 +3243,26 @@
* @param key the keyword by which the request is known
* (e.g., "<code>accept</code>").
* @param value the value associated with it.
- * @see #getRequestProperties(java.lang.String)
+ * @see #getRequestProperty(java.lang.String)
* @since 1.4
*/
@Override
- public synchronized void addRequestProperty(String key, String value) {
- if (connected || connecting)
- throw new IllegalStateException("Already connected");
- if (key == null)
- throw new NullPointerException ("key is null");
-
- if (isExternalMessageHeaderAllowed(key, value)) {
- requests.add(key, value);
- if (!key.equalsIgnoreCase("Content-Type")) {
+ public void addRequestProperty(String key, String value) {
+ connectionLock.lock();
+ try {
+ if (connected || connecting)
+ throw new IllegalStateException("Already connected");
+ if (key == null)
+ throw new NullPointerException("key is null");
+
+ if (isExternalMessageHeaderAllowed(key, value)) {
+ requests.add(key, value);
+ if (!key.equalsIgnoreCase("Content-Type")) {
userHeaders.add(key, value);
+ }
}
+ } finally {
+ connectionLock.unlock();
}
}
@@ -3215,31 +3271,43 @@
// the connected test.
//
public void setAuthenticationProperty(String key, String value) {
- checkMessageHeader(key, value);
- requests.set(key, value);
+ // use lock here to avoid the need for external synchronization
+ // in AuthenticationInfo subclasses
+ connectionLock.lock();
+ try {
+ checkMessageHeader(key, value);
+ requests.set(key, value);
+ } finally {
+ connectionLock.unlock();
+ }
}
@Override
- public synchronized String getRequestProperty (String key) {
- if (key == null) {
- return null;
- }
-
- // don't return headers containing security sensitive information
- for (int i=0; i < EXCLUDE_HEADERS.length; i++) {
- if (key.equalsIgnoreCase(EXCLUDE_HEADERS[i])) {
+ public String getRequestProperty (String key) {
+ connectionLock.lock();
+ try {
+ if (key == null) {
return null;
}
- }
- if (!setUserCookies) {
- if (key.equalsIgnoreCase("Cookie")) {
- return userCookies;
+
+ // don't return headers containing security sensitive information
+ for (int i = 0; i < EXCLUDE_HEADERS.length; i++) {
+ if (key.equalsIgnoreCase(EXCLUDE_HEADERS[i])) {
+ return null;
+ }
}
- if (key.equalsIgnoreCase("Cookie2")) {
- return userCookies2;
+ if (!setUserCookies) {
+ if (key.equalsIgnoreCase("Cookie")) {
+ return userCookies;
+ }
+ if (key.equalsIgnoreCase("Cookie2")) {
+ return userCookies2;
+ }
}
+ return requests.findValue(key);
+ } finally {
+ connectionLock.unlock();
}
- return requests.findValue(key);
}
/**
@@ -3255,29 +3323,34 @@
* @since 1.4
*/
@Override
- public synchronized Map<String, List<String>> getRequestProperties() {
- if (connected)
- throw new IllegalStateException("Already connected");
-
- // exclude headers containing security-sensitive info
- if (setUserCookies) {
- return requests.getHeaders(EXCLUDE_HEADERS);
+ public Map<String, List<String>> getRequestProperties() {
+ connectionLock.lock();
+ try {
+ if (connected)
+ throw new IllegalStateException("Already connected");
+
+ // exclude headers containing security-sensitive info
+ if (setUserCookies) {
+ return requests.getHeaders(EXCLUDE_HEADERS);
+ }
+ /*
+ * The cookies in the requests message headers may have
+ * been modified. Use the saved user cookies instead.
+ */
+ Map<String, List<String>> userCookiesMap = null;
+ if (userCookies != null || userCookies2 != null) {
+ userCookiesMap = new HashMap<>();
+ if (userCookies != null) {
+ userCookiesMap.put("Cookie", Arrays.asList(userCookies));
+ }
+ if (userCookies2 != null) {
+ userCookiesMap.put("Cookie2", Arrays.asList(userCookies2));
+ }
+ }
+ return requests.filterAndAddHeaders(EXCLUDE_HEADERS2, userCookiesMap);
+ } finally {
+ connectionLock.unlock();
}
- /*
- * The cookies in the requests message headers may have
- * been modified. Use the saved user cookies instead.
- */
- Map<String, List<String>> userCookiesMap = null;
- if (userCookies != null || userCookies2 != null) {
- userCookiesMap = new HashMap<>();
- if (userCookies != null) {
- userCookiesMap.put("Cookie", Arrays.asList(userCookies));
- }
- if (userCookies2 != null) {
- userCookiesMap.put("Cookie2", Arrays.asList(userCookies2));
- }
- }
- return requests.filterAndAddHeaders(EXCLUDE_HEADERS2, userCookiesMap);
}
@Override
@@ -3321,7 +3394,7 @@
* value to be used in milliseconds
* @throws IllegalArgumentException if the timeout parameter is negative
*
- * @see java.net.URLConnectiongetReadTimeout()
+ * @see java.net.URLConnection#getReadTimeout()
* @see java.io.InputStream#read()
* @since 1.5
*/
@@ -3440,6 +3513,7 @@
* @see java.io.FilterInputStream#in
* @see java.io.FilterInputStream#reset()
*/
+ // safe to use synchronized here: super method is synchronized too
@Override
public synchronized void mark(int readlimit) {
super.mark(readlimit);
@@ -3470,6 +3544,7 @@
* @see java.io.FilterInputStream#in
* @see java.io.FilterInputStream#mark(int)
*/
+ // safe to use synchronized here: super method is synchronized too
@Override
public synchronized void reset() throws IOException {
super.reset();