diff --git a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/FileBlobStorage.java b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/FileBlobStorage.java index 55effcd3b..36f5384d2 100644 --- a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/FileBlobStorage.java +++ b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/FileBlobStorage.java @@ -24,10 +24,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; /** * {@code FileBlobStorage} provides durable persistence of blobs by storing blob content directly to @@ -46,45 +42,17 @@ class FileBlobStorage implements BlobStorage { @Override public boolean hasBlob(final BlobKey blobKey) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Boolean run() { - return getFileForBlob(blobKey).exists(); - } - }); + return getFileForBlob(blobKey).exists(); } @Override public OutputStream storeBlob(final BlobKey blobKey) throws IOException { - try { - return AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public FileOutputStream run() throws IOException { - return new FileOutputStream(getFileForBlob(blobKey)); - } - }); - } catch (PrivilegedActionException ex) { - Throwable cause = ex.getCause(); - throw (cause instanceof IOException) ? (IOException) cause : new IOException(cause); - } + return new FileOutputStream(getFileForBlob(blobKey)); } @Override public InputStream fetchBlob(final BlobKey blobKey) throws IOException { - try { - return AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public FileInputStream run() throws IOException { - return new FileInputStream(getFileForBlob(blobKey)); - } - }); - } catch (PrivilegedActionException ex) { - Throwable cause = ex.getCause(); - throw (cause instanceof IOException) ? (IOException) cause : new IOException(cause); - } + return new FileInputStream(getFileForBlob(blobKey)); } @Override @@ -98,21 +66,9 @@ public void deleteBlob(final BlobKey blobKey) throws IOException { && blobInfoStorage.loadGsFileInfo(blobKey) == null) { throw new RuntimeException("Unknown blobkey: " + blobKey); } - try { - AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public Void run() throws IOException { - File file = getFileForBlob(blobKey); - if (!file.delete()) { - throw new IOException("Could not delete: " + file); - } - return null; - } - }); - } catch (PrivilegedActionException ex) { - Throwable cause = ex.getCause(); - throw (cause instanceof IOException) ? (IOException) cause : new IOException(cause); + File file = getFileForBlob(blobKey); + if (!file.delete()) { + throw new IOException("Could not delete: " + file); } blobInfoStorage.deleteBlobInfo(blobKey); } diff --git a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/LocalBlobstoreService.java b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/LocalBlobstoreService.java index aa51fbef1..9656c9671 100644 --- a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/LocalBlobstoreService.java +++ b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/LocalBlobstoreService.java @@ -43,8 +43,6 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; @@ -157,23 +155,18 @@ public CreateUploadURLResponse createUploadURL(Status status, CreateUploadURLReq } public VoidProto deleteBlob(Status status, final DeleteBlobRequest request) { - AccessController.doPrivileged( - (PrivilegedAction) - () -> { - for (String blobKeyString : request.getBlobKeyList()) { - BlobKey blobKey = new BlobKey(blobKeyString); - if (blobStorage.hasBlob(blobKey)) { - try { - blobStorage.deleteBlob(blobKey); - } catch (IOException ex) { - logger.log(Level.WARNING, "Could not delete blob: " + blobKey, ex); - throw new ApiProxy.ApplicationException( - BlobstoreServiceError.ErrorCode.INTERNAL_ERROR_VALUE, ex.toString()); - } - } - } - return null; - }); + for (String blobKeyString : request.getBlobKeyList()) { + BlobKey blobKey = new BlobKey(blobKeyString); + if (blobStorage.hasBlob(blobKey)) { + try { + blobStorage.deleteBlob(blobKey); + } catch (IOException ex) { + logger.log(Level.WARNING, "Could not delete blob: " + blobKey, ex); + throw new ApiProxy.ApplicationException( + BlobstoreServiceError.ErrorCode.INTERNAL_ERROR_VALUE, ex.toString()); + } + } + } return VoidProto.getDefaultInstance(); } @@ -222,27 +215,21 @@ public FetchDataResponse fetchData(Status status, final FetchDataRequest request } else { // Safe to cast because index will never be above MAX_BLOB_FETCH_SIZE. final byte[] data = new byte[(int) (endIndex - request.getStartIndex() + 1)]; - AccessController.doPrivileged( - (PrivilegedAction) - () -> { - try { - boolean swallowDueToThrow = true; - InputStream stream = blobStorage.fetchBlob(blobKey); - try { - ByteStreams.skipFully(stream, request.getStartIndex()); - ByteStreams.readFully(stream, data); - swallowDueToThrow = false; - } finally { - Closeables.close(stream, swallowDueToThrow); - } - } catch (IOException ex) { - logger.log(Level.WARNING, "Could not fetch data: " + blobKey, ex); - throw new ApiProxy.ApplicationException( - BlobstoreServiceError.ErrorCode.INTERNAL_ERROR_VALUE, ex.toString()); - } - - return null; - }); + try { + boolean swallowDueToThrow = true; + InputStream stream = blobStorage.fetchBlob(blobKey); + try { + ByteStreams.skipFully(stream, request.getStartIndex()); + ByteStreams.readFully(stream, data); + swallowDueToThrow = false; + } finally { + Closeables.close(stream, swallowDueToThrow); + } + } catch (IOException ex) { + logger.log(Level.WARNING, "Could not fetch data: " + blobKey, ex); + throw new ApiProxy.ApplicationException( + BlobstoreServiceError.ErrorCode.INTERNAL_ERROR_VALUE, ex.toString()); + } response.setData(ByteString.copyFrom(data)); } diff --git a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/UploadBlobServlet.java b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/UploadBlobServlet.java index d4166529d..dbfd1b082 100644 --- a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/UploadBlobServlet.java +++ b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/UploadBlobServlet.java @@ -34,11 +34,8 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; -import java.security.AccessController; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.security.SecureRandom; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -126,24 +123,7 @@ public void init() throws ServletException { @Override public void doPost(final HttpServletRequest req, final HttpServletResponse resp) throws ServletException, IOException { - try { - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public Object run() throws ServletException, IOException { - handleUpload(req, resp); - return null; - } - }); - } catch (PrivilegedActionException ex) { - Throwable cause = ex.getCause(); - if (cause instanceof ServletException) { - throw (ServletException) cause; - } else if (cause instanceof IOException) { - throw (IOException) cause; - } else { - throw new ServletException(cause); - } - } + handleUpload(req, resp); } private String getSessionId(HttpServletRequest req) { diff --git a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/ee10/UploadBlobServlet.java b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/ee10/UploadBlobServlet.java index 330109aab..28e085402 100644 --- a/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/ee10/UploadBlobServlet.java +++ b/api_dev/src/main/java/com/google/appengine/api/blobstore/dev/ee10/UploadBlobServlet.java @@ -47,11 +47,8 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; -import java.security.AccessController; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.security.SecureRandom; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -132,24 +129,7 @@ public void init() throws ServletException { @Override public void doPost(final HttpServletRequest req, final HttpServletResponse resp) throws ServletException, IOException { - try { - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public Object run() throws ServletException, IOException { - handleUpload(req, resp); - return null; - } - }); - } catch (PrivilegedActionException ex) { - Throwable cause = ex.getCause(); - if (cause instanceof ServletException) { - throw (ServletException) cause; - } else if (cause instanceof IOException) { - throw (IOException) cause; - } else { - throw new ServletException(cause); - } - } + handleUpload(req, resp); } private String getSessionId(HttpServletRequest req) { diff --git a/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalCompositeIndexManager.java b/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalCompositeIndexManager.java index d019eb11c..6e9d42693 100644 --- a/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalCompositeIndexManager.java +++ b/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalCompositeIndexManager.java @@ -52,8 +52,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.Writer; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; @@ -339,20 +337,13 @@ private File getGeneratedIndexFile() { /** * Returns an input stream for the generated indexes file or {@code null} if it doesn't exist. */ - // @Nullable // @VisibleForTesting - InputStream getGeneratedIndexFileInputStream() { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public InputStream run() { - try { - return new FileInputStream(getGeneratedIndexFile()); - } catch (FileNotFoundException e) { - return null; - } - } - }); + @Nullable InputStream getGeneratedIndexFileInputStream() { + try { + return new FileInputStream(getGeneratedIndexFile()); + } catch (FileNotFoundException e) { + return null; + } } /** Returns a writer for the generated indexes file. */ diff --git a/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalDatastoreService.java b/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalDatastoreService.java index 7f6986cb6..bcf91afb8 100644 --- a/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalDatastoreService.java +++ b/api_dev/src/main/java/com/google/appengine/api/datastore/dev/LocalDatastoreService.java @@ -114,12 +114,8 @@ import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.nio.charset.StandardCharsets; -import java.security.AccessController; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedAction; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -410,21 +406,14 @@ private static ScheduledThreadPoolExecutor createScheduler() { .setNameFormat("LocalDatastoreService-%d") .build()); scheduler.setRemoveOnCancelPolicy(true); - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Object run() { - Runtime.getRuntime() - .addShutdownHook( - new Thread() { - @Override - public void run() { - cleanupActiveServices(); - } - }); - return null; - } - }); + Runtime.getRuntime() + .addShutdownHook( + new Thread() { + @Override + public void run() { + cleanupActiveServices(); + } + }); return scheduler; } @@ -633,14 +622,7 @@ private static int parseInt(String valStr, int defaultVal, String propName) { } public void start() { - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Object run() { - startInternal(); - return null; - } - }); + startInternal(); } private synchronized void startInternal() { @@ -1402,16 +1384,8 @@ public boolean apply(EntityProto entity) { // store the query and return the results LiveQuery liveQuery = new LiveQuery(queryEntities, versions, query, entityComparator, clock); - // CompositeIndexManager does some filesystem reads/writes, so needs to - // be privileged. - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Object run() { - LocalCompositeIndexManager.getInstance().processQuery(validatedQuery.getV3Query()); - return null; - } - }); + // CompositeIndexManager does some filesystem reads/writes + LocalCompositeIndexManager.getInstance().processQuery(validatedQuery.getV3Query()); // Using next function to prefetch results and return them from runQuery QueryResult result = @@ -3224,32 +3198,25 @@ Map getSpecialPropertyMap() { private void persist() { globalLock.writeLock().lock(); try { - AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public Object run() throws IOException { - if (noStorage || !dirty) { - return null; - } + if (noStorage || !dirty) { + return; + } - long start = clock.getCurrentTime(); - try (ObjectOutputStream objectOut = - new ObjectOutputStream( - new BufferedOutputStream(new FileOutputStream(backingStore)))) { - objectOut.writeLong(-CURRENT_STORAGE_VERSION); - objectOut.writeLong(entityIdSequential.get()); - objectOut.writeLong(entityIdScattered.get()); - objectOut.writeObject(profiles); - } + long start = clock.getCurrentTime(); + try (ObjectOutputStream objectOut = + new ObjectOutputStream(new BufferedOutputStream(new FileOutputStream(backingStore)))) { + objectOut.writeLong(-CURRENT_STORAGE_VERSION); + objectOut.writeLong(entityIdSequential.get()); + objectOut.writeLong(entityIdScattered.get()); + objectOut.writeObject(profiles); + } - dirty = false; - long end = clock.getCurrentTime(); + dirty = false; + long end = clock.getCurrentTime(); - logger.log(Level.INFO, "Time to persist datastore: " + (end - start) + " ms"); - return null; - } - }); - } catch (PrivilegedActionException e) { + logger.log(Level.INFO, "Time to persist datastore: " + (end - start) + " ms"); + + } catch (Exception e) { Throwable t = e.getCause(); if (t instanceof IOException) { logger.log(Level.SEVERE, "Unable to save the datastore", e); diff --git a/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalBlobImageServlet.java b/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalBlobImageServlet.java index 593be79b8..6b8ab5e72 100644 --- a/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalBlobImageServlet.java +++ b/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalBlobImageServlet.java @@ -28,8 +28,6 @@ import java.awt.image.BufferedImage; import java.io.IOException; import java.io.OutputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -248,82 +246,74 @@ private ParsedUrl() { /** * Transforms the given image specified in the {@code ParseUrl} argument. * - * Applies all the requested resize and crop operations to a valid image. + *

Applies all the requested resize and crop operations to a valid image. * * @param request a valid {@code ParseUrl} instance - * * @return the transformed image in an Image class - * @throws ApiProxy.ApplicationException If the image cannot be opened, - * encoded, or if the transform is malformed + * @throws ApiProxy.ApplicationException If the image cannot be opened, encoded, or if the + * transform is malformed */ protected Image transformImage(final ParsedUrl request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Image run() { - // Obtain the image bytes as a BufferedImage - Status unusedStatus = new Status(); - ImageData imageData = - ImageData.newBuilder() - .setBlobKey(request.getBlobKey()) - .setContent(ByteString.EMPTY) - .build(); + // Obtain the image bytes as a BufferedImage + Status unusedStatus = new Status(); + ImageData imageData = + ImageData.newBuilder() + .setBlobKey(request.getBlobKey()) + .setContent(ByteString.EMPTY) + .build(); - String originalMimeType = imagesService.getMimeType(imageData); - BufferedImage img = imagesService.openImage(imageData, unusedStatus); + String originalMimeType = imagesService.getMimeType(imageData); + BufferedImage img = imagesService.openImage(imageData, unusedStatus); - // Apply the transform - if (request.hasOptions()) { - // Crop - if (request.getCrop()) { - Transform.Builder cropXform = null; - float width = img.getWidth(); - float height = img.getHeight(); - if (width > height) { - cropXform = Transform.newBuilder(); - float delta = (width - height) / (width * 2.0f); - cropXform.setCropLeftX(delta); - cropXform.setCropRightX(1.0f - delta); - } else if (width < height) { - cropXform = Transform.newBuilder(); - float delta = (height - width) / (height * 2.0f); - float topDelta = Math.max(0.0f, delta - 0.25f); - float bottomDelta = 1.0f - (2.0f * delta) + topDelta; - cropXform.setCropTopY(topDelta); - cropXform.setCropBottomY(bottomDelta); - } - if (cropXform != null) { - img = imagesService.processTransform(img, cropXform.build(), unusedStatus); - } - } + // Apply the transform + if (request.hasOptions()) { + // Crop + if (request.getCrop()) { + Transform.Builder cropXform = null; + float width = img.getWidth(); + float height = img.getHeight(); + if (width > height) { + cropXform = Transform.newBuilder(); + float delta = (width - height) / (width * 2.0f); + cropXform.setCropLeftX(delta); + cropXform.setCropRightX(1.0f - delta); + } else if (width < height) { + cropXform = Transform.newBuilder(); + float delta = (height - width) / (height * 2.0f); + float topDelta = Math.max(0.0f, delta - 0.25f); + float bottomDelta = 1.0f - (2.0f * delta) + topDelta; + cropXform.setCropTopY(topDelta); + cropXform.setCropBottomY(bottomDelta); + } + if (cropXform != null) { + img = imagesService.processTransform(img, cropXform.build(), unusedStatus); + } + } - // Resize - Transform resizeXform = - Transform.newBuilder() - .setWidth(request.getResize()) - .setHeight(request.getResize()) - .build(); - img = imagesService.processTransform(img, resizeXform, unusedStatus); - } else if (img.getWidth() > DEFAULT_SERVING_SIZE - || img.getHeight() > DEFAULT_SERVING_SIZE) { - // Resize down to default serving size. - Transform resizeXform = - Transform.newBuilder() - .setWidth(DEFAULT_SERVING_SIZE) - .setHeight(DEFAULT_SERVING_SIZE) - .build(); - img = imagesService.processTransform(img, resizeXform, unusedStatus); - } + // Resize + Transform resizeXform = + Transform.newBuilder() + .setWidth(request.getResize()) + .setHeight(request.getResize()) + .build(); + img = imagesService.processTransform(img, resizeXform, unusedStatus); + } else if (img.getWidth() > DEFAULT_SERVING_SIZE || img.getHeight() > DEFAULT_SERVING_SIZE) { + // Resize down to default serving size. + Transform resizeXform = + Transform.newBuilder() + .setWidth(DEFAULT_SERVING_SIZE) + .setHeight(DEFAULT_SERVING_SIZE) + .build(); + img = imagesService.processTransform(img, resizeXform, unusedStatus); + } - MIME_TYPE outputMimeType = MIME_TYPE.JPEG; - String outputMimeTypeString = "image/jpeg"; - if (transcodeToPng.contains(originalMimeType)) { - outputMimeType = MIME_TYPE.PNG; - outputMimeTypeString = "image/png"; - } - return new Image( - imagesService.saveImage(img, outputMimeType, unusedStatus), outputMimeTypeString); - } - }); + MIME_TYPE outputMimeType = MIME_TYPE.JPEG; + String outputMimeTypeString = "image/jpeg"; + if (transcodeToPng.contains(originalMimeType)) { + outputMimeType = MIME_TYPE.PNG; + outputMimeTypeString = "image/png"; + } + return new Image( + imagesService.saveImage(img, outputMimeType, unusedStatus), outputMimeTypeString); } } diff --git a/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalImagesService.java b/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalImagesService.java index 30c5603b0..6d45317dc 100644 --- a/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalImagesService.java +++ b/api_dev/src/main/java/com/google/appengine/api/images/dev/LocalImagesService.java @@ -61,8 +61,6 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -173,94 +171,88 @@ public void stop() {} */ public ImagesTransformResponse transform( final Status status, final ImagesTransformRequest request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public ImagesTransformResponse run() { - BufferedImage img = openImage(request.getImage(), status); - if (request.getTransformCount() > ImagesService.MAX_TRANSFORMS_PER_REQUEST) { - // TODO: Do we need to set both fields *and* throw an - // exception? - status.setSuccessful(false); - status.setErrorCode(ErrorCode.BAD_TRANSFORM_DATA.getNumber()); - throw new ApiProxy.ApplicationException( - ErrorCode.BAD_TRANSFORM_DATA.getNumber(), - String.format( - "%d transforms were supplied; the maximum allowed is %d.", - request.getTransformCount(), ImagesService.MAX_TRANSFORMS_PER_REQUEST)); - } - int orientation = 1; - if (request.getInput().getCorrectExifOrientation() - == ORIENTATION_CORRECTION_TYPE.CORRECT_ORIENTATION) { - Exif exif = getExifMetadata(request.getImage()); - if (exif != null) { - Entry entry = exif.getTagValue(Exif.ORIENTATION, true); - if (entry != null) { - orientation = ((Integer) entry.getValue(0)).intValue(); - if (img.getHeight() > img.getWidth()) { - orientation = 1; - } - } - } - } - for (Transform transform : request.getTransformList()) { - // In production, orientation correction is done during the first - // transform. If the first transform is a crop or flip it is done - // after, otherwise it is done before. To be precise, the order - // of transformation within a single entry is: Crop, Flip, - // Rotate, Resize, (Crop-to-fit), Effects (e.g., autolevels). - // Orientation fix is done within the chain modifying flipping - // and rotation steps. - if (orientation != 1 - && !(transform.hasCropRightX() - || transform.hasCropTopY() - || transform.hasCropBottomY() - || transform.hasCropLeftX()) - && !transform.hasHorizontalFlip() - && !transform.hasVerticalFlip()) { - img = correctOrientation(img, status, orientation); - orientation = 1; - } - if (transform.getAllowStretch() && transform.getCropToFit()) { - // Process allow stretch first and then process the crop. - // This is similar to how it works in production and allows us - // to keep the dev processing pipeline straightforward for this - // combination of transforms. - Transform.Builder stretch = Transform.newBuilder(); - stretch - .setWidth(transform.getWidth()) - .setHeight(transform.getHeight()) - .setAllowStretch(true); - img = processTransform(img, stretch.build(), status); - // Create and process the new crop portion of the transform. - Transform.Builder crop = Transform.newBuilder(); - crop.setWidth(transform.getWidth()) - .setHeight(transform.getHeight()) - .setCropToFit(transform.getCropToFit()) - .setCropOffsetX(transform.getCropOffsetX()) - .setCropOffsetY(transform.getCropOffsetY()) - .setAllowStretch(false); - img = processTransform(img, crop.build(), status); - } else { - img = processTransform(img, transform, status); - } - if (orientation != 1) { - img = correctOrientation(img, status, orientation); - orientation = 1; - } - } - status.setSuccessful(true); - ImageData imageData = - ImageData.newBuilder() - .setContent( - ByteString.copyFrom( - saveImage(img, request.getOutput().getMimeType(), status))) - .setWidth(img.getWidth()) - .setHeight(img.getHeight()) - .build(); - return ImagesTransformResponse.newBuilder().setImage(imageData).build(); + BufferedImage img = openImage(request.getImage(), status); + if (request.getTransformCount() > ImagesService.MAX_TRANSFORMS_PER_REQUEST) { + // TODO: Do we need to set both fields *and* throw an + // exception? + status.setSuccessful(false); + status.setErrorCode(ErrorCode.BAD_TRANSFORM_DATA.getNumber()); + throw new ApiProxy.ApplicationException( + ErrorCode.BAD_TRANSFORM_DATA.getNumber(), + String.format( + "%d transforms were supplied; the maximum allowed is %d.", + request.getTransformCount(), ImagesService.MAX_TRANSFORMS_PER_REQUEST)); + } + int orientation = 1; + if (request.getInput().getCorrectExifOrientation() + == ORIENTATION_CORRECTION_TYPE.CORRECT_ORIENTATION) { + Exif exif = getExifMetadata(request.getImage()); + if (exif != null) { + Entry entry = exif.getTagValue(Exif.ORIENTATION, true); + if (entry != null) { + orientation = ((Integer) entry.getValue(0)).intValue(); + if (img.getHeight() > img.getWidth()) { + orientation = 1; } - }); + } + } + } + for (Transform transform : request.getTransformList()) { + // In production, orientation correction is done during the first + // transform. If the first transform is a crop or flip it is done + // after, otherwise it is done before. To be precise, the order + // of transformation within a single entry is: Crop, Flip, + // Rotate, Resize, (Crop-to-fit), Effects (e.g., autolevels). + // Orientation fix is done within the chain modifying flipping + // and rotation steps. + if (orientation != 1 + && !(transform.hasCropRightX() + || transform.hasCropTopY() + || transform.hasCropBottomY() + || transform.hasCropLeftX()) + && !transform.hasHorizontalFlip() + && !transform.hasVerticalFlip()) { + img = correctOrientation(img, status, orientation); + orientation = 1; + } + if (transform.getAllowStretch() && transform.getCropToFit()) { + // Process allow stretch first and then process the crop. + // This is similar to how it works in production and allows us + // to keep the dev processing pipeline straightforward for this + // combination of transforms. + Transform.Builder stretch = Transform.newBuilder(); + stretch + .setWidth(transform.getWidth()) + .setHeight(transform.getHeight()) + .setAllowStretch(true); + img = processTransform(img, stretch.build(), status); + // Create and process the new crop portion of the transform. + Transform.Builder crop = Transform.newBuilder(); + crop.setWidth(transform.getWidth()) + .setHeight(transform.getHeight()) + .setCropToFit(transform.getCropToFit()) + .setCropOffsetX(transform.getCropOffsetX()) + .setCropOffsetY(transform.getCropOffsetY()) + .setAllowStretch(false); + img = processTransform(img, crop.build(), status); + } else { + img = processTransform(img, transform, status); + } + if (orientation != 1) { + img = correctOrientation(img, status, orientation); + orientation = 1; + } + } + status.setSuccessful(true); + ImageData imageData = + ImageData.newBuilder() + .setContent( + ByteString.copyFrom( + saveImage(img, request.getOutput().getMimeType(), status))) + .setWidth(img.getWidth()) + .setHeight(img.getHeight()) + .build(); + return ImagesTransformResponse.newBuilder().setImage(imageData).build(); } /** @@ -270,50 +262,44 @@ public ImagesTransformResponse run() { */ public ImagesCompositeResponse composite( final Status status, final ImagesCompositeRequest request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public ImagesCompositeResponse run() { - List images = new ArrayList(request.getImageCount()); - for (int i = 0; i < request.getImageCount(); i++) { - images.add(openImage(request.getImage(i), status)); - } - if (request.getOptionsCount() > ImagesService.MAX_COMPOSITES_PER_REQUEST) { - status.setSuccessful(false); - status.setErrorCode(ErrorCode.BAD_TRANSFORM_DATA.getNumber()); - throw new ApiProxy.ApplicationException(ErrorCode.BAD_TRANSFORM_DATA.getNumber(), - String.format("%d composites were supplied; the maximum allowed is %d.", - request.getOptionsCount(), ImagesService.MAX_COMPOSITES_PER_REQUEST)); - } - int width = request.getCanvas().getWidth(); - int height = request.getCanvas().getHeight(); - int color = request.getCanvas().getColor(); - BufferedImage canvas = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB); - for (int i = 0; i < height; i++) { - for (int j = 0; j < width; j++) { - canvas.setRGB(j, i, color); - } - } - for (int i = 0; i < request.getOptionsCount(); i++) { - CompositeImageOptions options = request.getOptions(i); - if (options.getSourceIndex() < 0 - || options.getSourceIndex() >= request.getImageCount()) { - throw new ApiProxy.ApplicationException(ErrorCode.BAD_TRANSFORM_DATA.getNumber(), - String.format("Invalid source image index %d", options.getSourceIndex())); - } - processComposite(canvas, options, images.get(options.getSourceIndex()), status); - } - status.setSuccessful(true); - return ImagesCompositeResponse - .newBuilder() - .setImage( - ImageData.newBuilder().setContent(ByteString.copyFrom(saveImage(canvas, request - .getCanvas() - .getOutput() - .getMimeType(), status)))) - .build(); - } - }); + List images = new ArrayList(request.getImageCount()); + for (int i = 0; i < request.getImageCount(); i++) { + images.add(openImage(request.getImage(i), status)); + } + if (request.getOptionsCount() > ImagesService.MAX_COMPOSITES_PER_REQUEST) { + status.setSuccessful(false); + status.setErrorCode(ErrorCode.BAD_TRANSFORM_DATA.getNumber()); + throw new ApiProxy.ApplicationException(ErrorCode.BAD_TRANSFORM_DATA.getNumber(), + String.format("%d composites were supplied; the maximum allowed is %d.", + request.getOptionsCount(), ImagesService.MAX_COMPOSITES_PER_REQUEST)); + } + int width = request.getCanvas().getWidth(); + int height = request.getCanvas().getHeight(); + int color = request.getCanvas().getColor(); + BufferedImage canvas = new BufferedImage(width, height, BufferedImage.TYPE_INT_ARGB); + for (int i = 0; i < height; i++) { + for (int j = 0; j < width; j++) { + canvas.setRGB(j, i, color); + } + } + for (int i = 0; i < request.getOptionsCount(); i++) { + CompositeImageOptions options = request.getOptions(i); + if (options.getSourceIndex() < 0 + || options.getSourceIndex() >= request.getImageCount()) { + throw new ApiProxy.ApplicationException(ErrorCode.BAD_TRANSFORM_DATA.getNumber(), + String.format("Invalid source image index %d", options.getSourceIndex())); + } + processComposite(canvas, options, images.get(options.getSourceIndex()), status); + } + status.setSuccessful(true); + return ImagesCompositeResponse + .newBuilder() + .setImage( + ImageData.newBuilder().setContent(ByteString.copyFrom(saveImage(canvas, request + .getCanvas() + .getOutput() + .getMimeType(), status)))) + .build(); } /** @@ -463,36 +449,30 @@ public byte[] saveImage(BufferedImage image, MIME_TYPE mimeType, Status status) */ public ImagesHistogramResponse histogram( final Status status, final ImagesHistogramRequest request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public ImagesHistogramResponse run() { - BufferedImage img = openImage(request.getImage(), status); - int[] red = new int[256]; - int[] green = new int[256]; - int[] blue = new int[256]; - int pixel; - for (int i = 0; i < img.getHeight(); i++) { - for (int j = 0; j < img.getWidth(); j++) { - pixel = img.getRGB(j, i); - // Premultiply by alpha to match thumbnailer. - red[(((pixel >> 16) & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; - green[(((pixel >> 8) & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; - blue[((pixel & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; - } - } - ImagesHistogram.Builder imageHistogram = ImagesHistogram.newBuilder(); - for (int i = 0; i < 256; i++) { - imageHistogram.addRed(red[i]); - imageHistogram.addGreen(green[i]); - imageHistogram.addBlue(blue[i]); - } - return ImagesHistogramResponse - .newBuilder() - .setHistogram(imageHistogram) - .build(); - } - }); + BufferedImage img = openImage(request.getImage(), status); + int[] red = new int[256]; + int[] green = new int[256]; + int[] blue = new int[256]; + int pixel; + for (int i = 0; i < img.getHeight(); i++) { + for (int j = 0; j < img.getWidth(); j++) { + pixel = img.getRGB(j, i); + // Premultiply by alpha to match thumbnailer. + red[(((pixel >> 16) & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; + green[(((pixel >> 8) & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; + blue[((pixel & 0xff) * ((pixel >> 24) & 0xff)) / 255]++; + } + } + ImagesHistogram.Builder imageHistogram = ImagesHistogram.newBuilder(); + for (int i = 0; i < 256; i++) { + imageHistogram.addRed(red[i]); + imageHistogram.addGreen(green[i]); + imageHistogram.addBlue(blue[i]); + } + return ImagesHistogramResponse + .newBuilder() + .setHistogram(imageHistogram) + .build(); } /** @@ -505,46 +485,34 @@ public ImagesHistogramResponse run() { */ public ImagesGetUrlBaseResponse getUrlBase( final Status status, final ImagesGetUrlBaseRequest request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public ImagesGetUrlBaseResponse run() { - if (request.getCreateSecureUrl()) { - log.info( - "Secure URLs will not be created using the development " + "application server."); - } - // Detect the image mimetype to see if is a valid image. - ImageData imageData = - ImageData.newBuilder() - .setBlobKey(request.getBlobKey()) - .setContent(ByteString.EMPTY) - .build(); - // getMimeType is validating the blob is an image. - getMimeType(imageData); - // Note I am commenting out the following line - // because experimentats indicates that doing so resolves - // b/7031367 Tests time out with OOMs since 1.7.1 - // TODO Figure out why the following line causes this - // test to take over one minute to finish: - // jt/c/g/dotorg/onetoday/server/offer/selection:FriendsMatchingScorerTest - // addServingUrlEntry(request.getBlobKey()); - return ImagesGetUrlBaseResponse.newBuilder() - .setUrl(hostPrefix + "/_ah/img/" + request.getBlobKey()) - .build(); - } - }); + if (request.getCreateSecureUrl()) { + log.info( + "Secure URLs will not be created using the development " + "application server."); + } + // Detect the image mimetype to see if is a valid image. + ImageData imageData = + ImageData.newBuilder() + .setBlobKey(request.getBlobKey()) + .setContent(ByteString.EMPTY) + .build(); + // getMimeType is validating the blob is an image. + getMimeType(imageData); + // Note I am commenting out the following line + // because experimentats indicates that doing so resolves + // b/7031367 Tests time out with OOMs since 1.7.1 + // TODO Figure out why the following line causes this + // test to take over one minute to finish: + // jt/c/g/dotorg/onetoday/server/offer/selection:FriendsMatchingScorerTest + // addServingUrlEntry(request.getBlobKey()); + return ImagesGetUrlBaseResponse.newBuilder() + .setUrl(hostPrefix + "/_ah/img/" + request.getBlobKey()) + .build(); } public ImagesDeleteUrlBaseResponse deleteUrlBase( final Status status, final ImagesDeleteUrlBaseRequest request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public ImagesDeleteUrlBaseResponse run() { - deleteServingUrlEntry(request.getBlobKey()); - return ImagesDeleteUrlBaseResponse.newBuilder().build(); - } - }); + deleteServingUrlEntry(request.getBlobKey()); + return ImagesDeleteUrlBaseResponse.newBuilder().build(); } @Override diff --git a/api_dev/src/main/java/com/google/appengine/api/images/dev/ee10/LocalBlobImageServlet.java b/api_dev/src/main/java/com/google/appengine/api/images/dev/ee10/LocalBlobImageServlet.java index f93433f42..ca2e18e27 100644 --- a/api_dev/src/main/java/com/google/appengine/api/images/dev/ee10/LocalBlobImageServlet.java +++ b/api_dev/src/main/java/com/google/appengine/api/images/dev/ee10/LocalBlobImageServlet.java @@ -33,8 +33,6 @@ import java.awt.image.BufferedImage; import java.io.IOException; import java.io.OutputStream; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -249,82 +247,74 @@ private ParsedUrl() { /** * Transforms the given image specified in the {@code ParseUrl} argument. * - * Applies all the requested resize and crop operations to a valid image. + *

Applies all the requested resize and crop operations to a valid image. * * @param request a valid {@code ParseUrl} instance - * * @return the transformed image in an Image class - * @throws ApiProxy.ApplicationException If the image cannot be opened, - * encoded, or if the transform is malformed + * @throws ApiProxy.ApplicationException If the image cannot be opened, encoded, or if the + * transform is malformed */ protected Image transformImage(final ParsedUrl request) { - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Image run() { - // Obtain the image bytes as a BufferedImage - Status unusedStatus = new Status(); - ImageData imageData = - ImageData.newBuilder() - .setBlobKey(request.getBlobKey()) - .setContent(ByteString.EMPTY) - .build(); + // Obtain the image bytes as a BufferedImage + Status unusedStatus = new Status(); + ImageData imageData = + ImageData.newBuilder() + .setBlobKey(request.getBlobKey()) + .setContent(ByteString.EMPTY) + .build(); - String originalMimeType = imagesService.getMimeType(imageData); - BufferedImage img = imagesService.openImage(imageData, unusedStatus); + String originalMimeType = imagesService.getMimeType(imageData); + BufferedImage img = imagesService.openImage(imageData, unusedStatus); - // Apply the transform - if (request.hasOptions()) { - // Crop - if (request.getCrop()) { - Transform.Builder cropXform = null; - float width = img.getWidth(); - float height = img.getHeight(); - if (width > height) { - cropXform = Transform.newBuilder(); - float delta = (width - height) / (width * 2.0f); - cropXform.setCropLeftX(delta); - cropXform.setCropRightX(1.0f - delta); - } else if (width < height) { - cropXform = Transform.newBuilder(); - float delta = (height - width) / (height * 2.0f); - float topDelta = Math.max(0.0f, delta - 0.25f); - float bottomDelta = 1.0f - (2.0f * delta) + topDelta; - cropXform.setCropTopY(topDelta); - cropXform.setCropBottomY(bottomDelta); - } - if (cropXform != null) { - img = imagesService.processTransform(img, cropXform.build(), unusedStatus); - } - } + // Apply the transform + if (request.hasOptions()) { + // Crop + if (request.getCrop()) { + Transform.Builder cropXform = null; + float width = img.getWidth(); + float height = img.getHeight(); + if (width > height) { + cropXform = Transform.newBuilder(); + float delta = (width - height) / (width * 2.0f); + cropXform.setCropLeftX(delta); + cropXform.setCropRightX(1.0f - delta); + } else if (width < height) { + cropXform = Transform.newBuilder(); + float delta = (height - width) / (height * 2.0f); + float topDelta = Math.max(0.0f, delta - 0.25f); + float bottomDelta = 1.0f - (2.0f * delta) + topDelta; + cropXform.setCropTopY(topDelta); + cropXform.setCropBottomY(bottomDelta); + } + if (cropXform != null) { + img = imagesService.processTransform(img, cropXform.build(), unusedStatus); + } + } - // Resize - Transform resizeXform = - Transform.newBuilder() - .setWidth(request.getResize()) - .setHeight(request.getResize()) - .build(); - img = imagesService.processTransform(img, resizeXform, unusedStatus); - } else if (img.getWidth() > DEFAULT_SERVING_SIZE - || img.getHeight() > DEFAULT_SERVING_SIZE) { - // Resize down to default serving size. - Transform resizeXform = - Transform.newBuilder() - .setWidth(DEFAULT_SERVING_SIZE) - .setHeight(DEFAULT_SERVING_SIZE) - .build(); - img = imagesService.processTransform(img, resizeXform, unusedStatus); - } + // Resize + Transform resizeXform = + Transform.newBuilder() + .setWidth(request.getResize()) + .setHeight(request.getResize()) + .build(); + img = imagesService.processTransform(img, resizeXform, unusedStatus); + } else if (img.getWidth() > DEFAULT_SERVING_SIZE || img.getHeight() > DEFAULT_SERVING_SIZE) { + // Resize down to default serving size. + Transform resizeXform = + Transform.newBuilder() + .setWidth(DEFAULT_SERVING_SIZE) + .setHeight(DEFAULT_SERVING_SIZE) + .build(); + img = imagesService.processTransform(img, resizeXform, unusedStatus); + } - MIME_TYPE outputMimeType = MIME_TYPE.JPEG; - String outputMimeTypeString = "image/jpeg"; - if (transcodeToPng.contains(originalMimeType)) { - outputMimeType = MIME_TYPE.PNG; - outputMimeTypeString = "image/png"; - } - return new Image( - imagesService.saveImage(img, outputMimeType, unusedStatus), outputMimeTypeString); - } - }); + MIME_TYPE outputMimeType = MIME_TYPE.JPEG; + String outputMimeTypeString = "image/jpeg"; + if (transcodeToPng.contains(originalMimeType)) { + outputMimeType = MIME_TYPE.PNG; + outputMimeTypeString = "image/png"; + } + return new Image( + imagesService.saveImage(img, outputMimeType, unusedStatus), outputMimeTypeString); } } diff --git a/api_dev/src/main/java/com/google/appengine/api/search/dev/LocalSearchService.java b/api_dev/src/main/java/com/google/appengine/api/search/dev/LocalSearchService.java index 099d441a3..7a62b1e2a 100644 --- a/api_dev/src/main/java/com/google/appengine/api/search/dev/LocalSearchService.java +++ b/api_dev/src/main/java/com/google/appengine/api/search/dev/LocalSearchService.java @@ -48,9 +48,6 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.nio.charset.StandardCharsets; -import java.security.AccessController; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -843,17 +840,11 @@ private void clearIndexes(final File indexDirectory) { } else { closeIndexWriters(); try { - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public Object run() throws IOException { - if (indexDirectory.exists()) { - recursiveDelete(indexDirectory); - } - indexDirectory.mkdirs(); - return null; - } - }); - } catch (PrivilegedActionException e) { + if (indexDirectory.exists()) { + recursiveDelete(indexDirectory); + } + indexDirectory.mkdirs(); + } catch (IOException e) { throw new RuntimeException(e); } dirMap = new LuceneDirectoryMap.FileBased(indexDirectory); diff --git a/api_dev/src/main/java/com/google/appengine/api/taskqueue/dev/LocalTaskQueue.java b/api_dev/src/main/java/com/google/appengine/api/taskqueue/dev/LocalTaskQueue.java index 0c2ea1020..eff4593ba 100644 --- a/api_dev/src/main/java/com/google/appengine/api/taskqueue/dev/LocalTaskQueue.java +++ b/api_dev/src/main/java/com/google/appengine/api/taskqueue/dev/LocalTaskQueue.java @@ -57,8 +57,6 @@ import java.net.Inet6Address; import java.net.InetAddress; import java.nio.file.Paths; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Collections; import java.util.HashMap; import java.util.IdentityHashMap; @@ -272,14 +270,7 @@ void setQueueXml(QueueXml queueXml) { @Override public void start() { - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Object run() { - start_(); - return null; - } - }); + start_(); } private void start_() { @@ -347,14 +338,7 @@ static String getBaseUrl(LocalServerEnvironment localServerEnvironment) { public void stop() { // Avoid removing the shutdownHook while a JVM shutdown is in progress. if (shutdownHook != null) { - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Void run() { - Runtime.getRuntime().removeShutdownHook(shutdownHook); - return null; - } - }); + Runtime.getRuntime().removeShutdownHook(shutdownHook); shutdownHook = null; } stop_(); diff --git a/api_dev/src/main/java/com/google/appengine/api/urlfetch/dev/LocalURLFetchService.java b/api_dev/src/main/java/com/google/appengine/api/urlfetch/dev/LocalURLFetchService.java index 6ee62cdee..997e11700 100644 --- a/api_dev/src/main/java/com/google/appengine/api/urlfetch/dev/LocalURLFetchService.java +++ b/api_dev/src/main/java/com/google/appengine/api/urlfetch/dev/LocalURLFetchService.java @@ -35,13 +35,10 @@ import java.net.ProxySelector; import java.net.SocketTimeoutException; import java.net.URL; -import java.security.AccessController; import java.security.KeyManagementException; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.util.Arrays; @@ -465,43 +462,26 @@ private HttpResponse doPrivilegedExecute( final HttpRequestBase method, final URLFetchResponse.Builder response) throws IOException { - try { - return AccessController.doPrivileged( - new PrivilegedExceptionAction() { - @Override - public HttpResponse run() throws IOException { - HttpContext context = new BasicHttpContext(); - // Does some thread ops we need to do in a privileged block. - HttpResponse httpResponse; - // TODO: Default behavior reverted to not validating cert for - // 1.4.2 CP due to wildcard cert validation problems. Revert for - // 1.4.4 after we're confident that the new HttpClient has fixed the - // behavior. - if (request.hasMustValidateServerCertificate() - && request.getMustValidateServerCertificate()) { - httpResponse = getValidatingClient().execute(method, context); - } else { - httpResponse = getNonValidatingClient().execute(method, context); - } - response.setStatusCode(httpResponse.getStatusLine().getStatusCode()); - HttpHost lastHost = - (HttpHost) context.getAttribute(ExecutionContext.HTTP_TARGET_HOST); - HttpUriRequest lastReq = - (HttpUriRequest) context.getAttribute(ExecutionContext.HTTP_REQUEST); - String lastUrl = lastHost.toURI() + lastReq.getURI(); - if (!lastUrl.equals(method.getURI().toString())) { - response.setFinalUrl(lastUrl); - } - return httpResponse; - } - }); - } catch (PrivilegedActionException e) { - Throwable t = e.getCause(); - if (t instanceof IOException) { - throw (IOException) t; - } - throw new RuntimeException(e); + HttpContext context = new BasicHttpContext(); + // Does some thread ops we need to do in a privileged block. + HttpResponse httpResponse; + // TODO: Default behavior reverted to not validating cert for + // 1.4.2 CP due to wildcard cert validation problems. Revert for + // 1.4.4 after we're confident that the new HttpClient has fixed the + // behavior. + if (request.hasMustValidateServerCertificate() && request.getMustValidateServerCertificate()) { + httpResponse = getValidatingClient().execute(method, context); + } else { + httpResponse = getNonValidatingClient().execute(method, context); + } + response.setStatusCode(httpResponse.getStatusLine().getStatusCode()); + HttpHost lastHost = (HttpHost) context.getAttribute(ExecutionContext.HTTP_TARGET_HOST); + HttpUriRequest lastReq = (HttpUriRequest) context.getAttribute(ExecutionContext.HTTP_REQUEST); + String lastUrl = lastHost.toURI() + lastReq.getURI(); + if (!lastUrl.equals(method.getURI().toString())) { + response.setFinalUrl(lastUrl); } + return httpResponse; } boolean isAllowedPort(int port) { diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ApiProxyLocalImpl.java b/api_dev/src/main/java/com/google/appengine/tools/development/ApiProxyLocalImpl.java index 9304d3b12..09e7a9035 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ApiProxyLocalImpl.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ApiProxyLocalImpl.java @@ -26,8 +26,6 @@ import com.google.apphosting.api.ApiProxy.UnknownException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -42,8 +40,6 @@ import java.util.concurrent.Future; import java.util.concurrent.Semaphore; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; import org.checkerframework.checker.nullness.qual.Nullable; @@ -216,15 +212,9 @@ public Future makeAsyncCall( boolean offline = environment.getAttributes().get(IS_OFFLINE_REQUEST_KEY) != null; boolean success = false; try { - // Despite the name, privilegedCallable() just arranges for this - // callable to be run with the current privileges. - Callable callable = Executors.privilegedCallable(asyncApiCall); - - // Now we need to escalate privileges so we have permission to - // spin up new threads, if necessary. The callable itself will - // run with the previous privileges. - Future resultFuture = AccessController.doPrivileged( - new PrivilegedApiAction(callable, asyncApiCall)); + Callable callable = asyncApiCall; + Future resultFuture = apiExecutor.submit(callable); + success = true; if (context.getLocalServerEnvironment().enforceApiDeadlines()) { long deadlineMillis = (long) (1000.0 * resolveDeadline(packageName, apiConfig, offline)); @@ -274,67 +264,6 @@ private double resolveDeadline( return Math.min(deadline, maxDeadline); } - private class PrivilegedApiAction implements PrivilegedAction> { - - private final Callable callable; - private final AsyncApiCall asyncApiCall; - - PrivilegedApiAction(Callable callable, AsyncApiCall asyncApiCall) { - this.callable = callable; - this.asyncApiCall = asyncApiCall; - } - - @Override - public Future run() { - // TODO: Return something that implements - // ApiProxy.ApiResultFuture so we can attach real wallclock - // time information here (although CPU time is irrelevant). - final Future result = apiExecutor.submit(callable); - return new Future() { - @Override - public boolean cancel(final boolean mayInterruptIfRunning) { - // Cancel may interrupt another thread so we need to escalate privileges to avoid - // sandbox restrictions. - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Boolean run() { - // If we cancel the task before it runs it's up to us to - // release the semaphore. If we cancel the task after it - // runs we know the task released the semaphore. However, - // we can't reliably know the state of the task and it's - // bad news if the semaphore gets released twice. This - // method ensures that the semaphore only gets released once. - asyncApiCall.tryReleaseSemaphore(); - return result.cancel(mayInterruptIfRunning); - } - }); - } - - @Override - public boolean isCancelled() { - return result.isCancelled(); - } - - @Override - public boolean isDone() { - return result.isDone(); - } - - @Override - public byte[] get() throws InterruptedException, ExecutionException { - return result.get(); - } - - @Override - public byte[] get(long timeout, TimeUnit unit) - throws InterruptedException, ExecutionException, TimeoutException { - return result.get(timeout, unit); - } - }; - } - } - @Override public void setProperty(String serviceProperty, String value) { if (serviceProperty == null) { @@ -568,13 +497,7 @@ public final synchronized LocalRpcService getService(final String pkg) { return cachedService; } - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public LocalRpcService run() { - return startServices(pkg); - } - }); + return startServices(pkg); } @Override diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/BackgroundThreadFactory.java b/api_dev/src/main/java/com/google/appengine/tools/development/BackgroundThreadFactory.java index 6ae5be95f..01315d635 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/BackgroundThreadFactory.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/BackgroundThreadFactory.java @@ -17,8 +17,6 @@ package com.google.appengine.tools.development; import com.google.apphosting.api.ApiProxy; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.concurrent.ThreadFactory; import java.util.logging.Level; import java.util.logging.Logger; @@ -55,28 +53,22 @@ public Thread newThread(final Runnable runnable) { LocalEnvironment.getCurrentInstance(), LocalEnvironment.getCurrentPort()); sleepUninterruptably(API_CALL_LATENCY_MS); - return AccessController.doPrivileged( - new PrivilegedAction() { + // TODO: Only allow this to be used from a backend. + Thread thread = + new Thread(runnable) { @Override - public Thread run() { - // TODO: Only allow this to be used from a backend. - Thread thread = - new Thread(runnable) { - @Override - public void run() { - sleepUninterruptably(THREAD_STARTUP_LATENCY_MS); - ApiProxy.setEnvironmentForCurrentThread(environment); - try { - runnable.run(); - } finally { - environment.callRequestEndListeners(); - } - } - }; - System.setProperty("devappserver-thread-" + thread.getName(), "true"); - return thread; + public void run() { + sleepUninterruptably(THREAD_STARTUP_LATENCY_MS); + ApiProxy.setEnvironmentForCurrentThread(environment); + try { + runnable.run(); + } finally { + environment.callRequestEndListeners(); + } } - }); + }; + System.setProperty("devappserver-thread-" + thread.getName(), "true"); + return thread; } final String getAppId() { diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/DevAppServerImpl.java b/api_dev/src/main/java/com/google/appengine/tools/development/DevAppServerImpl.java index 034297ba1..dc20912ab 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/DevAppServerImpl.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/DevAppServerImpl.java @@ -29,10 +29,6 @@ import com.google.common.collect.ImmutableSet; import java.io.File; import java.net.BindException; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.HashMap; import java.util.Map; import java.util.TimeZone; @@ -222,23 +218,14 @@ public Map getServiceProperties() { /** * Starts the server. * - * @throws IllegalStateException If the server has already been started or - * shutdown. + * @throws IllegalStateException If the server has already been started or shutdown. * @throws AppEngineConfigException If no WEB-INF directory can be found or - * WEB-INF/appengine-web.xml does not exist. + * WEB-INF/appengine-web.xml does not exist. * @return a latch that will be decremented to zero when the server is shutdown. */ @Override public CountDownLatch start() throws Exception { - try { - return AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override public CountDownLatch run() throws Exception { - return doStart(); - } - }); - } catch (PrivilegedActionException e) { - throw e.getException(); - } + return doStart(); } private CountDownLatch doStart() throws Exception { @@ -374,24 +361,16 @@ public CountDownLatch restart() throws Exception { if (serverState != ServerState.RUNNING) { throw new IllegalStateException("Cannot restart a server that is not currently running."); } - try { - return AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override public CountDownLatch run() throws Exception { - modules.shutdown(); - backendContainer.shutdownAll(); - shutdownLatch.countDown(); - modules.createConnections(); - backendContainer.configureAll(apiProxyLocal); - modules.setApiProxyDelegate(apiProxyLocal); - modules.startup(); - backendContainer.startupAll(); - shutdownLatch = new CountDownLatch(1); - return shutdownLatch; - } - }); - } catch (PrivilegedActionException e) { - throw e.getException(); - } + modules.shutdown(); + backendContainer.shutdownAll(); + shutdownLatch.countDown(); + modules.createConnections(); + backendContainer.configureAll(apiProxyLocal); + modules.setApiProxyDelegate(apiProxyLocal); + modules.startup(); + backendContainer.startupAll(); + shutdownLatch = new CountDownLatch(1); + return shutdownLatch; } @Override @@ -399,46 +378,29 @@ public void shutdown() throws Exception { if (serverState != ServerState.RUNNING) { throw new IllegalStateException("Cannot shutdown a server that is not currently running."); } - try { - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override public Void run() throws Exception { - modules.shutdown(); - backendContainer.shutdownAll(); - ApiProxy.setDelegate(null); - apiProxyLocal = null; - serverState = ServerState.SHUTDOWN; - shutdownLatch.countDown(); - return null; - } - }); - } catch (PrivilegedActionException e) { - throw e.getException(); - } + modules.shutdown(); + backendContainer.shutdownAll(); + ApiProxy.setDelegate(null); + apiProxyLocal = null; + serverState = ServerState.SHUTDOWN; + shutdownLatch.countDown(); } @Override public void gracefulShutdown() throws IllegalStateException { // TODO: Do an actual graceful shutdown rather than just delaying. - // Requires a privileged block since this may be invoked from a servlet - // that lives in the user's classloader and may result in the creation of - // a thread. - AccessController.doPrivileged( - new PrivilegedAction>() { - @Override - public Future run() { - return shutdownScheduler.schedule( - new Callable() { - @Override - public Void call() throws Exception { - shutdown(); - return null; - } - }, - 1000, - TimeUnit.MILLISECONDS); - } - }); + Future unused = + shutdownScheduler.schedule( + new Callable() { + @Override + public Void call() throws Exception { + shutdown(); + return null; + } + }, + 1000, + TimeUnit.MILLISECONDS); } @Override diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/IsolatedAppClassLoader.java b/api_dev/src/main/java/com/google/appengine/tools/development/IsolatedAppClassLoader.java index dfa5c3436..9ed4723d5 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/IsolatedAppClassLoader.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/IsolatedAppClassLoader.java @@ -26,9 +26,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; -import java.security.AccessController; import java.security.CodeSource; -import java.security.PrivilegedAction; import java.util.HashSet; import java.util.Set; import java.util.logging.Level; @@ -172,13 +170,7 @@ protected synchronized Class loadClass(String name, boolean resolve) final Class c = devAppServerClassLoader.loadClass(name); // See where it came from. - CodeSource source = AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public CodeSource run() { - return c.getProtectionDomain().getCodeSource(); - } - }); + CodeSource source = c.getProtectionDomain().getCodeSource(); // Load classes from the JRE. // We can't just block non-allowlisted classes from being loaded. The JVM diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/ManualInstanceHolder.java b/api_dev/src/main/java/com/google/appengine/tools/development/ManualInstanceHolder.java index c0453d200..48289b84d 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/ManualInstanceHolder.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/ManualInstanceHolder.java @@ -19,8 +19,6 @@ import com.google.appengine.tools.development.ApplicationConfigurationManager.ModuleConfigurationHandle; import com.google.appengine.tools.development.InstanceStateHolder.InstanceState; import java.io.File; -import java.security.AccessController; -import java.security.PrivilegedExceptionAction; import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -157,18 +155,7 @@ void stopServing() throws Exception { startRequestLatch = new CountDownLatch(1); doConfigure(); createConnection(); - // We call ContainerService.startup inside a PrivilegedExceptionAction - // so threads created by the contained Jetty instance - // will not inherit our callers protection domains. See - // http://docs.oracle.com/javase/7/docs/technotes/guides/security/spec/security-spec.doc4.html - // section 4.3 for details. - AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public Object run() throws Exception { - getContainerService().startup(); - return null; - } - }); + getContainerService().startup(); stateHolder.testAndSet(InstanceState.STOPPED, InstanceState.INITIALIZING); } } diff --git a/api_dev/src/main/java/com/google/appengine/tools/development/RequestThreadFactory.java b/api_dev/src/main/java/com/google/appengine/tools/development/RequestThreadFactory.java index e59f0ced7..28b3f966c 100644 --- a/api_dev/src/main/java/com/google/appengine/tools/development/RequestThreadFactory.java +++ b/api_dev/src/main/java/com/google/appengine/tools/development/RequestThreadFactory.java @@ -17,9 +17,6 @@ package com.google.appengine.tools.development; import com.google.apphosting.api.ApiProxy; -import java.security.AccessControlContext; -import java.security.AccessController; -import java.security.PrivilegedAction; import java.util.Date; import java.util.Map; import java.util.concurrent.ThreadFactory; @@ -44,105 +41,93 @@ public class RequestThreadFactory implements ThreadFactory { @Override public Thread newThread(final Runnable runnable) { - final AccessControlContext context = AccessController.getContext(); - return AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Thread run() { - final ApiProxy.Environment environment = ApiProxy.getCurrentEnvironment(); - Thread thread = - new Thread() { - /** - * If the thread is started, install a {@link RequestEndListener} to interrupt the - * thread at the end of the request. We don't yet enforce request deadlines in the - * DevAppServer so we don't need to handle other interrupt cases yet. - */ + final ApiProxy.Environment environment = ApiProxy.getCurrentEnvironment(); + + Thread thread = + new Thread() { + /** + * If the thread is started, install a {@link RequestEndListener} to interrupt the + * thread at the end of the request. We don't yet enforce request deadlines in the + * DevAppServer so we don't need to handle other interrupt cases yet. + */ + @Override + public synchronized void start() { + try { + Thread.sleep(THREAD_STARTUP_LATENCY_MS); + } catch (InterruptedException ex) { + // We can't propagate the exception from here so + // just log, reset the bit, and continue. + logger.log( + Level.INFO, "Interrupted while simulating thread startup latency", ex); + Thread.currentThread().interrupt(); + } + super.start(); + final Thread thread = this; // Thread.this doesn't work from an anon subclass + RequestEndListenerHelper.register( + new RequestEndListener() { @Override - public synchronized void start() { - try { - Thread.sleep(THREAD_STARTUP_LATENCY_MS); - } catch (InterruptedException ex) { - // We can't propagate the exception from here so - // just log, reset the bit, and continue. - logger.log( - Level.INFO, "Interrupted while simulating thread startup latency", ex); - Thread.currentThread().interrupt(); + public void onRequestEnd(ApiProxy.Environment environment) { + if (thread.isAlive()) { + logger.info("Interrupting request thread: " + thread); + thread.interrupt(); + logger.info("Waiting up to 100ms for thread to complete: " + thread); + try { + thread.join(100); + } catch (InterruptedException ex) { + logger.info("Interrupted while waiting."); + } + if (thread.isAlive()) { + logger.info("Interrupting request thread again: " + thread); + thread.interrupt(); + long remaining = getRemainingDeadlineMillis(environment); + logger.info( + "Waiting up to " + + remaining + + " ms for thread to complete: " + + thread); + try { + thread.join(remaining); + } catch (InterruptedException ex) { + logger.info("Interrupted while waiting."); + } + if (thread.isAlive()) { + Throwable stack = new Throwable(); + stack.setStackTrace(thread.getStackTrace()); + logger.log( + Level.SEVERE, + "Thread left running: " + + thread + + ". " + + "In production this will cause the request to fail.", + stack); + } + } } - super.start(); - final Thread thread = this; // Thread.this doesn't work from an anon subclass - RequestEndListenerHelper.register( - new RequestEndListener() { - @Override - public void onRequestEnd(ApiProxy.Environment environment) { - if (thread.isAlive()) { - logger.info("Interrupting request thread: " + thread); - thread.interrupt(); - logger.info("Waiting up to 100ms for thread to complete: " + thread); - try { - thread.join(100); - } catch (InterruptedException ex) { - logger.info("Interrupted while waiting."); - } - if (thread.isAlive()) { - logger.info("Interrupting request thread again: " + thread); - thread.interrupt(); - long remaining = getRemainingDeadlineMillis(environment); - logger.info( - "Waiting up to " - + remaining - + " ms for thread to complete: " - + thread); - try { - thread.join(remaining); - } catch (InterruptedException ex) { - logger.info("Interrupted while waiting."); - } - if (thread.isAlive()) { - Throwable stack = new Throwable(); - stack.setStackTrace(thread.getStackTrace()); - logger.log( - Level.SEVERE, - "Thread left running: " - + thread - + ". " - + "In production this will cause the request to fail.", - stack); - } - } - } - } - }); } + }); + } - @Override - public void run() { - // Copy the current environment to the new thread. - ApiProxy.setEnvironmentForCurrentThread(environment); - // Switch back to the calling context before running the user's code. - AccessController.doPrivileged( - new PrivilegedAction() { - @Override - public Object run() { - runnable.run(); - return null; - } - }, - context); - // Don't bother unsetting the environment. We're - // not going to reuse this thread and we want the - // environment still to be set during any - // UncaughtExceptionHandler (which happens after - // run() completes/throws). - } - }; - // This system property is used to check if the thread is - // running user code (ugly, I know). This thread is now - // running user code so we set it as well. - System.setProperty("devappserver-thread-" + thread.getName(), "true"); - return thread; + @Override + public void run() { + // Copy the current environment to the new thread. + ApiProxy.setEnvironmentForCurrentThread(environment); + // Switch back to the calling context before running the user's code. + runnable.run(); + // Don't bother unsetting the environment. We're + // not going to reuse this thread and we want the + // environment still to be set during any + // UncaughtExceptionHandler (which happens after + // run() completes/throws). } - }); + }; + // This system property is used to check if the thread is + // running user code (ugly, I know). This thread is now + // running user code so we set it as well. + System.setProperty("devappserver-thread-" + thread.getName(), "true"); + return thread; + + } private long getRemainingDeadlineMillis(ApiProxy.Environment environment) {