Changeset 9227 in josm for trunk/src

Timestamp:

2015-12-31T03:24:56+01:00 (9 years ago)

Author:

Don-vip

Message:

OAuth: add robustness, basic unit test, code cleanup

Location:

trunk/src

Files:

• oauth/signpost/AbstractOAuthProvider.java (modified) (10 diffs)
• org/openstreetmap/josm/gui/oauth/OsmLoginFailedException.java (modified) (1 diff)
• org/openstreetmap/josm/gui/oauth/OsmOAuthAuthorizationClient.java (modified) (3 diffs)
• org/openstreetmap/josm/gui/oauth/OsmOAuthAuthorizationException.java (modified) (1 diff)
• org/openstreetmap/josm/io/ProgressInputStream.java (modified) (4 diffs)
• org/openstreetmap/josm/tools/HttpClient.java (modified) (2 diffs)

trunk/src/oauth/signpost/AbstractOAuthProvider.java

@@ -12 +12 @@
 
 import java.io.BufferedReader;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.util.HashMap;
@@ -28 +29 @@
  * you will probably inherit from this class, since it takes a lot of work from
  * you.
- * 
+ *
  * @author Matthias Kaeppler
  */
@@ -128 +129 @@
      * </ul>
      * </p>
-     * 
+     *
      * @param consumer
      *        the {@link OAuthConsumer} that should be used to sign the request
@@ -168 +169 @@
                 consumer.setAdditionalParameters(customOAuthParams);
             }
-            
+
             if (this.listener != null) {
                 this.listener.prepareRequest(request);
@@ -174 +175 @@
 
             consumer.sign(request);
-            
+
             if (this.listener != null) {
                 this.listener.prepareSubmission(request);
@@ -230 +231 @@
             return;
         }
-        BufferedReader reader = new BufferedReader(new InputStreamReader(response.getContent()));
         StringBuilder responseBody = new StringBuilder();
-
-        String line = reader.readLine();
-        while (line != null) {
-            responseBody.append(line);
-            line = reader.readLine();
+        InputStream content = response.getContent();
+        if (content != null) {
+            BufferedReader reader = new BufferedReader(new InputStreamReader(content));
+
+            String line = reader.readLine();
+            while (line != null) {
+                responseBody.append(line);
+                line = reader.readLine();
+            }
         }
 
@@ -251 +255 @@
      * Overrride this method if you want to customize the logic for building a
      * request object for the given endpoint URL.
-     * 
+     *
      * @param endpointUrl
      *        the URL to which the request will go
@@ -263 +267 @@
      * Override this method if you want to customize the logic for how the given
      * request is sent to the server.
-     * 
+     *
      * @param request
      *        the request to send
@@ -275 +279 @@
      * Called when the connection is being finalized after receiving the
      * response. Use this to do any cleanup / resource freeing.
-     * 
+     *
      * @param request
      *        the request that has been sent
@@ -295 +299 @@
      * token reply. You must call {@link #setResponseParameters} with the set of
      * parameters before using this method.
-     * 
+     *
      * @param key
      *        the parameter name

trunk/src/org/openstreetmap/josm/gui/oauth/OsmLoginFailedException.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.oauth;
 
+/**
+ * OSM login failure exception.
+ * @since 2746
+ */
 public class OsmLoginFailedException extends OsmOAuthAuthorizationException {
 
-    public OsmLoginFailedException() {
-        super();
-    }
-
-    public OsmLoginFailedException(String arg0, Throwable arg1) {
-        super(arg0, arg1);
-    }
-
-    public OsmLoginFailedException(String arg0) {
-        super(arg0);
-    }
-
-    public OsmLoginFailedException(Throwable arg0) {
-        super(arg0);
+    /**
+     * Constructs a new {@code OsmLoginFailedException} with the specified cause.
+     * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method).
+     */
+    public OsmLoginFailedException(Throwable cause) {
+        super(cause);
     }
 }

trunk/src/org/openstreetmap/josm/gui/oauth/OsmOAuthAuthorizationClient.java

@@ -50 +50 @@
         private String token;
         private String userName;
-    }
-
-    /**
-     * Creates a new authorisation client with default OAuth parameters
-     *
-     */
-    public OsmOAuthAuthorizationClient() {
-        oauthProviderParameters = OAuthParameters.createDefault(Main.pref.get("osm-server.url"));
-        consumer = oauthProviderParameters.buildConsumer();
-        provider = oauthProviderParameters.buildProvider(consumer);
     }
 
@@ -241 +231 @@
     }
 
-    protected String buildPostRequest(Map<String, String> parameters) throws OsmOAuthAuthorizationException {
+    protected static String buildPostRequest(Map<String, String> parameters) {
         StringBuilder sb = new StringBuilder(32);
 
@@ -322 +312 @@
      * Submits a request to the OSM website for a OAuth form. The OSM website replies a session token in
      * a hidden parameter.
+     * @param sessionId session id
+     * @param requestToken request token
      *
      * @throws OsmOAuthAuthorizationException if something went wrong

trunk/src/org/openstreetmap/josm/gui/oauth/OsmOAuthAuthorizationException.java

@@ -2 +2 @@
 package org.openstreetmap.josm.gui.oauth;
 
+/**
+ * OSM OAuth authorization exception.
+ * @since 2746
+ */
 public class OsmOAuthAuthorizationException extends Exception {
 
-    public OsmOAuthAuthorizationException() {
-        super();
+    /**
+     * Constructs a new {@code OsmLoginFailedException} with the specified detail message.
+     * @param message the detail message. The detail message is saved for later retrieval by the {@link #getMessage()} method.
+     */
+    public OsmOAuthAuthorizationException(String message) {
+        super(message);
     }
 
-    public OsmOAuthAuthorizationException(String arg0, Throwable arg1) {
-        super(arg0, arg1);
-    }
-
-    public OsmOAuthAuthorizationException(String arg0) {
-        super(arg0);
-    }
-
-    public OsmOAuthAuthorizationException(Throwable arg0) {
-        super(arg0);
+    /**
+     * Constructs a new {@code OsmLoginFailedException} with the specified cause.
+     * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method).
+     */
+    public OsmOAuthAuthorizationException(Throwable cause) {
+        super(cause);
     }
 }

trunk/src/org/openstreetmap/josm/io/ProgressInputStream.java

@@ -10 +10 @@
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
+import org.openstreetmap.josm.tools.CheckParameterUtil;
 
 /**
@@ -23 +24 @@
      * Constructs a new {@code ProgressInputStream}.
      *
-     * @param in the stream to monitor
+     * @param in the stream to monitor. Must not be null
      * @param size the total size which will be sent
      * @param progressMonitor the monitor to report to
@@ -29 +30 @@
      */
     public ProgressInputStream(InputStream in, long size, ProgressMonitor progressMonitor) {
+        CheckParameterUtil.ensureParameterNotNull(in, "in");
         if (progressMonitor == null) {
             progressMonitor = NullProgressMonitor.INSTANCE;
@@ -43 +45 @@
      * @param con the connection to monitor
      * @param progressMonitor the monitor to report to
+     * @throws OsmTransferException if any I/O error occurs
      */
     public ProgressInputStream(URLConnection con, ProgressMonitor progressMonitor) throws OsmTransferException {

trunk/src/org/openstreetmap/josm/tools/HttpClient.java

@@ -231 +231 @@
          * @see HttpURLConnection#getErrorStream()
          */
+        @SuppressWarnings("resource")
         public InputStream getContent() throws IOException {
             InputStream in;
@@ -238 +239 @@
                 in = connection.getErrorStream();
             }
-            in = new ProgressInputStream(in, getContentLength(), monitor);
-            in = "gzip".equalsIgnoreCase(getContentEncoding()) ? new GZIPInputStream(in) : in;
-            Compression compression = Compression.NONE;
-            if (uncompress) {
-                final String contentType = getContentType();
-                Main.debug("Uncompressing input stream according to Content-Type header: {0}", contentType);
-                compression = Compression.forContentType(contentType);
-            }
-            if (uncompressAccordingToContentDisposition && Compression.NONE.equals(compression)) {
-                final String contentDisposition = getHeaderField("Content-Disposition");
-                final Matcher matcher = Pattern.compile("filename=\"([^\"]+)\"").matcher(contentDisposition);
-                if (matcher.find()) {
-                    Main.debug("Uncompressing input stream according to Content-Disposition header: {0}", contentDisposition);
-                    compression = Compression.byExtension(matcher.group(1));
+            if (in != null) {
+                in = new ProgressInputStream(in, getContentLength(), monitor);
+                in = "gzip".equalsIgnoreCase(getContentEncoding()) ? new GZIPInputStream(in) : in;
+                Compression compression = Compression.NONE;
+                if (uncompress) {
+                    final String contentType = getContentType();
+                    Main.debug("Uncompressing input stream according to Content-Type header: {0}", contentType);
+                    compression = Compression.forContentType(contentType);
                 }
-            }
-            in = compression.getUncompressedInputStream(in);
+                if (uncompressAccordingToContentDisposition && Compression.NONE.equals(compression)) {
+                    final String contentDisposition = getHeaderField("Content-Disposition");
+                    final Matcher matcher = Pattern.compile("filename=\"([^\"]+)\"").matcher(contentDisposition);
+                    if (matcher.find()) {
+                        Main.debug("Uncompressing input stream according to Content-Disposition header: {0}", contentDisposition);
+                        compression = Compression.byExtension(matcher.group(1));
+                    }
+                }
+                in = compression.getUncompressedInputStream(in);
+            }
             return in;
         }