- Timestamp:
- 2024-08-19T20:16:02+02:00 (8 months ago)
- Location:
- trunk
- Files:
-
- 4 edited
-
src/org/openstreetmap/josm/io/remotecontrol/RequestProcessor.java (modified) (5 diffs)
-
src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java (modified) (6 diffs)
-
src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java (modified) (3 diffs)
-
test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java (modified) (6 diffs)
Legend:
- Unmodified
- Added
- Removed
-
trunk/src/org/openstreetmap/josm/io/remotecontrol/RequestProcessor.java
r18723 r19200 21 21 import java.util.StringTokenizer; 22 22 import java.util.TreeMap; 23 import java.util.concurrent.locks.ReentrantLock; 23 24 import java.util.regex.Matcher; 24 25 import java.util.regex.Pattern; … … 54 55 public class RequestProcessor extends Thread { 55 56 57 /** This is purely used to ensure that remote control commands are executed in the order in which they are received */ 58 private static final ReentrantLock ORDER_LOCK = new ReentrantLock(true); 56 59 private static final Charset RESPONSE_CHARSET = StandardCharsets.UTF_8; 57 60 private static final String RESPONSE_TEMPLATE = "<!DOCTYPE html><html><head><meta charset=\"" … … 182 185 @Override 183 186 public void run() { 184 Writer out = null; // NOPMD 185 try { // NOPMD 186 out = new OutputStreamWriter(new BufferedOutputStream(request.getOutputStream()), RESPONSE_CHARSET); 187 BufferedReader in = new BufferedReader(new InputStreamReader(request.getInputStream(), StandardCharsets.US_ASCII)); // NOPMD 188 187 // The locks ensure that we process the instructions in the order in which they came. 188 // This is mostly important when the caller is attempting to create a new layer and add multiple download 189 // instructions for that layer. See #23821 for additional details. 190 ORDER_LOCK.lock(); 191 try (request; 192 Writer out = new OutputStreamWriter(new BufferedOutputStream(request.getOutputStream()), RESPONSE_CHARSET); 193 BufferedReader in = new BufferedReader(new InputStreamReader(request.getInputStream(), StandardCharsets.US_ASCII))) { 194 realRun(in, out, request); 195 } catch (IOException ioe) { 196 Logging.debug(Logging.getErrorMessage(ioe)); 197 } finally { 198 ORDER_LOCK.unlock(); 199 } 200 } 201 202 /** 203 * Perform the actual commands 204 * @param in The reader for incoming data 205 * @param out The writer for outgoing data 206 * @param request The actual request 207 * @throws IOException Usually occurs if one of the {@link Writer} methods has problems. 208 */ 209 private static void realRun(BufferedReader in, Writer out, Socket request) throws IOException { 210 try { 189 211 String get = in.readLine(); 190 212 if (get == null) { … … 211 233 } 212 234 213 int questionPos = url.indexOf('?'); 214 215 String command = questionPos < 0 ? url : url.substring(0, questionPos); 216 217 Map<String, String> headers = new HashMap<>(); 218 int k = 0; 219 int maxHeaders = 20; 220 while (k < maxHeaders) { 221 get = in.readLine(); 222 if (get == null) break; 223 k++; 224 String[] h = get.split(": ", 2); 225 if (h.length == 2) { 226 headers.put(h[0], h[1]); 227 } else break; 228 } 229 230 // Who sent the request: trying our best to detect 231 // not from localhost => sender = IP 232 // from localhost: sender = referer header, if exists 233 String sender = null; 234 235 if (!request.getInetAddress().isLoopbackAddress()) { 236 sender = request.getInetAddress().getHostAddress(); 237 } else { 238 String ref = headers.get("Referer"); 239 Pattern r = Pattern.compile("(https?://)?([^/]*)"); 240 if (ref != null) { 241 Matcher m = r.matcher(ref); 242 if (m.find()) { 243 sender = m.group(2); 244 } 245 } 246 if (sender == null) { 247 sender = "localhost"; 248 } 249 } 250 251 // find a handler for this command 252 Class<? extends RequestHandler> handlerClass = handlers.get(command); 253 if (handlerClass == null) { 254 String usage = getUsageAsHtml(); 255 String websiteDoc = HelpUtil.getWikiBaseHelpUrl() +"/Help/Preferences/RemoteControl"; 256 String help = "No command specified! The following commands are available:<ul>" + usage 257 + "</ul>" + "See <a href=\""+websiteDoc+"\">"+websiteDoc+"</a> for complete documentation."; 258 sendErrorHtml(out, 400, "Bad Request", help); 259 } else { 260 // create handler object 261 RequestHandler handler = handlerClass.getConstructor().newInstance(); 262 try { 263 handler.setCommand(command); 264 handler.setUrl(url); 265 handler.setSender(sender); 266 handler.handle(); 267 sendHeader(out, "200 OK", handler.getContentType(), false); 268 out.write("Content-length: " + handler.getContent().length() 269 + "\r\n"); 270 out.write("\r\n"); 271 out.write(handler.getContent()); 272 out.flush(); 273 } catch (RequestHandlerOsmApiException ex) { 274 Logging.debug(ex); 275 sendBadGateway(out, ex.getMessage()); 276 } catch (RequestHandlerErrorException ex) { 277 Logging.debug(ex); 278 sendInternalError(out, ex.getMessage()); 279 } catch (RequestHandlerBadRequestException ex) { 280 Logging.debug(ex); 281 sendBadRequest(out, ex.getMessage()); 282 } catch (RequestHandlerForbiddenException ex) { 283 Logging.debug(ex); 284 sendForbidden(out, ex.getMessage()); 285 } 286 } 287 } catch (IOException ioe) { 288 Logging.debug(Logging.getErrorMessage(ioe)); 235 final int questionPos = url.indexOf('?'); 236 237 final String command = questionPos < 0 ? url : url.substring(0, questionPos); 238 239 final Map<String, String> headers = parseHeaders(in); 240 final String sender = parseSender(headers, request); 241 callHandler(url, command, out, sender); 289 242 } catch (ReflectiveOperationException e) { 290 243 Logging.error(e); … … 294 247 Logging.warn(e1); 295 248 } 296 } finally { 249 } 250 } 251 252 /** 253 * Parse the headers from the request 254 * @param in The request reader 255 * @return The map of headers 256 * @throws IOException See {@link BufferedReader#readLine()} 257 */ 258 private static Map<String, String> parseHeaders(BufferedReader in) throws IOException { 259 Map<String, String> headers = new HashMap<>(); 260 int k = 0; 261 int maxHeaders = 20; 262 int lastSize = -1; 263 while (k < maxHeaders && lastSize != headers.size()) { 264 lastSize = headers.size(); 265 String get = in.readLine(); 266 if (get != null) { 267 k++; 268 String[] h = get.split(": ", 2); 269 if (h.length == 2) { 270 headers.put(h[0], h[1]); 271 } 272 } 273 } 274 return headers; 275 } 276 277 /** 278 * Attempt to figure out who sent the request 279 * @param headers The headers (we currently look for {@code Referer}) 280 * @param request The request to look at 281 * @return The sender (or {@code "localhost"} if none could be found) 282 */ 283 private static String parseSender(Map<String, String> headers, Socket request) { 284 // Who sent the request: trying our best to detect 285 // not from localhost => sender = IP 286 // from localhost: sender = referer header, if exists 287 if (!request.getInetAddress().isLoopbackAddress()) { 288 return request.getInetAddress().getHostAddress(); 289 } 290 String ref = headers.get("Referer"); 291 Pattern r = Pattern.compile("(https?://)?([^/]*)"); 292 if (ref != null) { 293 Matcher m = r.matcher(ref); 294 if (m.find()) { 295 return m.group(2); 296 } 297 } 298 return "localhost"; 299 } 300 301 /** 302 * Call the handler for the command 303 * @param url The request URL 304 * @param command The command we are using 305 * @param out The writer to use for indicating success or failure 306 * @param sender The sender of the request 307 * @throws ReflectiveOperationException If the handler class has an issue 308 * @throws IOException If one of the {@link Writer} methods has issues 309 */ 310 private static void callHandler(String url, String command, Writer out, String sender) throws ReflectiveOperationException, IOException { 311 // find a handler for this command 312 Class<? extends RequestHandler> handlerClass = handlers.get(command); 313 if (handlerClass == null) { 314 String usage = getUsageAsHtml(); 315 String websiteDoc = HelpUtil.getWikiBaseHelpUrl() +"/Help/Preferences/RemoteControl"; 316 String help = "No command specified! The following commands are available:<ul>" + usage 317 + "</ul>" + "See <a href=\""+websiteDoc+"\">"+websiteDoc+"</a> for complete documentation."; 318 sendErrorHtml(out, 400, "Bad Request", help); 319 } else { 320 // create handler object 321 RequestHandler handler = handlerClass.getConstructor().newInstance(); 297 322 try { 298 request.close(); 299 } catch (IOException e) { 300 Logging.debug(Logging.getErrorMessage(e)); 323 handler.setCommand(command); 324 handler.setUrl(url); 325 handler.setSender(sender); 326 handler.handle(); 327 sendHeader(out, "200 OK", handler.getContentType(), false); 328 out.write("Content-length: " + handler.getContent().length() 329 + "\r\n"); 330 out.write("\r\n"); 331 out.write(handler.getContent()); 332 out.flush(); 333 } catch (RequestHandlerOsmApiException ex) { 334 Logging.debug(ex); 335 sendBadGateway(out, ex.getMessage()); 336 } catch (RequestHandlerErrorException ex) { 337 Logging.debug(ex); 338 sendInternalError(out, ex.getMessage()); 339 } catch (RequestHandlerBadRequestException ex) { 340 Logging.debug(ex); 341 sendBadRequest(out, ex.getMessage()); 342 } catch (RequestHandlerForbiddenException ex) { 343 Logging.debug(ex); 344 sendForbidden(out, ex.getMessage()); 301 345 } 302 346 } -
trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java
r19153 r19200 8 8 import java.util.Collection; 9 9 import java.util.Collections; 10 import java.util.HashMap;11 10 import java.util.LinkedHashSet; 12 11 import java.util.List; … … 17 16 import java.util.concurrent.TimeUnit; 18 17 import java.util.concurrent.TimeoutException; 19 import java.util.concurrent.locks.ReadWriteLock;20 import java.util.concurrent.locks.ReentrantReadWriteLock;21 18 import java.util.stream.Collectors; 22 19 import java.util.stream.Stream; … … 76 73 private static final String SEARCH = "search"; 77 74 78 private static final Map<String, ReadWriteLock> layerLockMap = new HashMap<>();79 80 75 // Mandatory arguments 81 76 private double minlat; … … 166 161 private void download() throws RequestHandlerErrorException { 167 162 DownloadOsmTask osmTask = new DownloadOsmTask(); 168 ReadWriteLock lock = null;169 DownloadParams settings = null;170 163 try { 171 settings = getDownloadParams(); 164 final DownloadParams settings = getDownloadParams(); 172 165 173 166 if (command.equals(myCommand)) { … … 175 168 Logging.info("RemoteControl: download forbidden by preferences"); 176 169 } else { 177 // The lock ensures that a download that creates a new layer will finish before178 // downloads that do not create a new layer. This should be thread-safe.179 lock = obtainLock(settings);180 // We need to ensure that we only try to download new areas181 170 Area toDownload = null; 182 171 if (!settings.isNewLayer()) { … … 190 179 } 191 180 } 192 } catch (InterruptedException ex) {193 Thread.currentThread().interrupt();194 throw new RequestHandlerErrorException(ex);195 181 } catch (RuntimeException ex) { // NOPMD 196 182 Logging.warn("RemoteControl: Error parsing load_and_zoom remote control request:"); 197 183 Logging.error(ex); 198 184 throw new RequestHandlerErrorException(ex); 199 } finally {200 releaseLock(settings, lock);201 }202 }203 204 /**205 * Obtain a lock to ensure that a new layer is created before downloading non-new layers206 * @param settings The settings with the appropriate layer name; if no layer name is given, we assume that207 * the caller doesn't care where the data goes.208 * @return The lock to pass to {@link #releaseLock(DownloadParams, ReadWriteLock)} or {@code null} if no lock is needed.209 * @throws InterruptedException If the lock could not be obtained.210 */211 private static ReadWriteLock obtainLock(DownloadParams settings) throws InterruptedException {212 final ReadWriteLock lock;213 if (settings.isNewLayer() && !Utils.isEmpty(settings.getLayerName())) {214 synchronized (layerLockMap) {215 lock = layerLockMap.computeIfAbsent(settings.getLayerName(), k -> new ReentrantReadWriteLock());216 lock.writeLock().lock();217 }218 } else {219 synchronized (layerLockMap) {220 lock = layerLockMap.get(settings.getLayerName());221 }222 if (lock != null) {223 lock.readLock().lockInterruptibly();224 }225 }226 return lock;227 }228 229 /**230 * Release the lock preventing data from being downloaded into an old layer231 * @param settings The settings with information on the new layer status232 * @param lock The lock to unlock233 */234 private static void releaseLock(DownloadParams settings, ReadWriteLock lock) {235 if (lock != null) {236 if (settings != null && settings.isNewLayer()) {237 lock.writeLock().unlock();238 synchronized (layerLockMap) {239 layerLockMap.remove(settings.getLayerName());240 }241 } else {242 lock.readLock().unlock();243 }244 185 } 245 186 } -
trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/RequestHandler.java
r19196 r19200 14 14 import java.util.Map; 15 15 import java.util.Set; 16 import java.util.concurrent.locks.ReentrantLock;17 16 import java.util.function.Function; 18 17 import java.util.function.Supplier; … … 49 48 /** preference to define OSM download timeout in seconds */ 50 49 public static final IntegerProperty OSM_DOWNLOAD_TIMEOUT = new IntegerProperty("remotecontrol.osm.download.timeout", 5*60); 51 /** A lock to ensure that messages are shown in the order the commands were sent from the server (see #23821) */52 private static final ReentrantLock MESSAGE_LOCK = new ReentrantLock(true);53 50 54 51 protected static final Pattern SPLITTER_COMMA = Pattern.compile(",\\s*"); … … 212 209 final Object[] choices = {tr("Yes, always"), tr("Yes, once"), tr("No")}; 213 210 final int choice; 214 // The ordering of the requests can be important, so we use a fair lock to ensure ordering 215 // Note that the EDT will be more than happy to show multiple dialogs without blocking. 216 try { 217 MESSAGE_LOCK.lock(); 218 final Integer tChoice = GuiHelper.runInEDTAndWaitAndReturn(() -> { 219 final JLabel label = new JLabel(message); 220 if (label.getPreferredSize().width > maxWidth) { 221 label.setText(message.replaceFirst("<div>", "<div style=\"width:" + maxWidth + "px;\">")); 222 } 223 return JOptionPane.showOptionDialog(MainApplication.getMainFrame(), label, tr("Confirm Remote Control action"), 224 JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.QUESTION_MESSAGE, null, choices, choices[1]); 225 }); 226 if (tChoice == null) { 227 // I have no clue how this would ever happen, but just in case. 228 throw new RequestHandlerForbiddenException(MessageFormat.format("RemoteControl: ''{0}'' forbidden due to NPE", myCommand)); 211 final Integer tChoice = GuiHelper.runInEDTAndWaitAndReturn(() -> { 212 final JLabel label = new JLabel(message); 213 if (label.getPreferredSize().width > maxWidth) { 214 label.setText(message.replaceFirst("<div>", "<div style=\"width:" + maxWidth + "px;\">")); 229 215 } 230 choice = tChoice; 231 } finally { 232 MESSAGE_LOCK.unlock(); 216 return JOptionPane.showOptionDialog(MainApplication.getMainFrame(), label, tr("Confirm Remote Control action"), 217 JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.QUESTION_MESSAGE, null, choices, choices[1]); 218 }); 219 if (tChoice == null) { 220 // I have no clue how this would ever happen, but just in case. 221 throw new RequestHandlerForbiddenException(MessageFormat.format("RemoteControl: ''{0}'' forbidden due to NPE", myCommand)); 233 222 } 223 choice = tChoice; 234 224 if (choice != JOptionPane.YES_OPTION && choice != JOptionPane.NO_OPTION) { // Yes/no refer to always/once 235 225 String err = MessageFormat.format("RemoteControl: ''{0}'' forbidden by user''s choice", myCommand); -
trunk/test/unit/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandlerTest.java
r19153 r19200 11 11 import static org.junit.jupiter.api.Assertions.assertTrue; 12 12 13 import java.io.IOException; 14 import java.net.URI; 13 15 import java.net.URLEncoder; 14 16 import java.nio.charset.StandardCharsets; … … 31 33 import org.openstreetmap.josm.gui.layer.OsmDataLayer; 32 34 import org.openstreetmap.josm.gui.util.GuiHelper; 35 import org.openstreetmap.josm.io.remotecontrol.RemoteControl; 33 36 import org.openstreetmap.josm.io.remotecontrol.handler.RequestHandler.RequestHandlerBadRequestException; 37 import org.openstreetmap.josm.spi.preferences.Config; 34 38 import org.openstreetmap.josm.testutils.annotations.BasicPreferences; 35 39 … … 39 43 import org.openstreetmap.josm.testutils.annotations.Projection; 40 44 import org.openstreetmap.josm.testutils.annotations.ThreadSync; 45 import org.openstreetmap.josm.tools.HttpClient; 41 46 42 47 /** … … 47 52 @ExtendWith(BasicWiremock.OsmApiExtension.class) 48 53 class LoadAndZoomHandlerTest { 49 private static final String DEFAULT_BBOX_URL = "http s://localhost/load_and_zoom?left=0&bottom=0&right=0.001&top=0.001";50 private static final String DEFAULT_BBOX_URL_2 = "http s://localhost/load_and_zoom?left=0.00025&bottom=0.00025&right=0.00075&top=0.00125";54 private static final String DEFAULT_BBOX_URL = "http://localhost/load_and_zoom?left=0&bottom=0&right=0.001&top=0.001"; 55 private static final String DEFAULT_BBOX_URL_2 = "http://localhost/load_and_zoom?left=0.00025&bottom=0.00025&right=0.00075&top=0.00125"; 51 56 private static LoadAndZoomHandler newHandler(String url) throws RequestHandlerBadRequestException { 52 57 LoadAndZoomHandler req = new LoadAndZoomHandler(); … … 237 242 /** 238 243 * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/23821">#23821</a> 239 * @throws RequestHandlerBadRequestException If there is an issue with the handler 240 */ 241 @Test 242 void testNonRegression23821() throws RequestHandlerBadRequestException { 244 * @param wireMockRuntimeInfo The runtime info 245 */ 246 @Test 247 void testNonRegression23821(WireMockRuntimeInfo wireMockRuntimeInfo) throws IOException { 248 // Start remote control on a random port to avoid failing when JOSM is open. 249 final int port = wireMockRuntimeInfo.getHttpPort() + 1; 250 Config.getPref().putInt("remote.control.port", port); 243 251 final AtomicBoolean block = new AtomicBoolean(false); 244 252 final Runnable runnable = () -> { … … 257 265 MainApplication.getLayerManager().addLayer(new OsmDataLayer(wrongDataset, 258 266 "LoadAndZoomHandlerTest#testNonRegression23821", null)); 259 ForkJoinTask<?> task1;260 ForkJoinTask<?> task2;261 267 try { 262 GuiHelper.runInEDT(runnable); 263 MainApplication.worker.submit(runnable); 264 // The processor makes a new handler for each request 265 // It is single-threaded, so blocking on one handler would fix the problem with the other handler. 266 // But we might as well work on multi-threading, since it is easier to test. :) 267 final LoadAndZoomHandler handler1 = newHandler(DEFAULT_BBOX_URL + "&new_layer=true&layer_name=OSMData"); 268 final LoadAndZoomHandler handler2 = newHandler(DEFAULT_BBOX_URL_2 + "&new_layer=false&layer_name=OSMData"); 269 // Use a separate threads to avoid blocking on this thread 270 final ForkJoinPool pool = ForkJoinPool.commonPool(); 271 task1 = pool.submit(() -> assertDoesNotThrow(handler1::handle)); 272 273 // Make certain there is enough time for the first task to block 274 Awaitility.await().until(() -> true); 275 task2 = pool.submit(() -> assertDoesNotThrow(handler2::handle)); 268 ForkJoinTask<?> task1; 269 ForkJoinTask<?> task2; 270 try { 271 GuiHelper.runInEDT(runnable); 272 MainApplication.worker.submit(runnable); 273 // The processor makes a new handler for each request 274 // It is single-threaded, so blocking on one handler would fix the problem with the other handler. 275 // But we might as well work on multi-threading, since it is easier to test. :) 276 final HttpClient first = HttpClient.create(URI.create(DEFAULT_BBOX_URL.replace("localhost", "localhost:" + port) 277 + "&new_layer=true&layer_name=OSMData").toURL()); 278 final HttpClient second = HttpClient.create(URI.create(DEFAULT_BBOX_URL_2.replace("localhost", "localhost:" + port) 279 + "&new_layer=false&layer_name=OSMData").toURL()); 280 // Use a separate threads to avoid blocking on this thread. 281 final ForkJoinPool pool = ForkJoinPool.commonPool(); 282 RemoteControl.start(); 283 task1 = pool.submit(() -> assertDoesNotThrow(() -> { 284 try { 285 assertEquals("OK\r\n", first.connect().fetchContent()); 286 } finally { 287 first.disconnect(); 288 } 289 })); 290 // Make certain there is enough time for the first task to block 291 Awaitility.await().until(() -> true); 292 task2 = pool.submit(() -> assertDoesNotThrow(() -> { 293 try { 294 assertEquals("OK\r\n", second.connect().fetchContent()); 295 } finally { 296 second.disconnect(); 297 } 298 })); 299 } finally { 300 // Unblock UI/worker threads 301 synchronized (block) { 302 block.set(true); 303 block.notifyAll(); 304 } 305 } 306 307 task1.join(); 308 task2.join(); 309 310 syncThreads(); 276 311 } finally { 277 // Unblock UI/worker threads 278 synchronized (block) { 279 block.set(true); 280 block.notifyAll(); 281 } 312 // We have to stop remote control _after_ we join the tasks and sync threads. 313 RemoteControl.stop(); 282 314 } 283 284 task1.join();285 task2.join();286 287 syncThreads();288 315 assertEquals(2, MainApplication.getLayerManager().getLayers().size()); 289 316 final DataSet ds = MainApplication.getLayerManager().getEditDataSet();
Note:
See TracChangeset
for help on using the changeset viewer.
