-
Notifications
You must be signed in to change notification settings - Fork 682
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dev 1 #358
base: master
Are you sure you want to change the base?
Dev 1 #358
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package com.mainpackage; | ||
|
||
import com.amazonaws.services.s3.AmazonS3; | ||
import com.amazonaws.services.s3.model.S3Object; | ||
import java.io.IOException; | ||
import java.io.InputStream; | ||
import com.amazonaws.util.IOUtils; | ||
|
||
/** | ||
* Even though this file contains file syntax issues, CodeGuru Reviewer will not | ||
* report any issues in it, because it has been excluded in aws-codeguru-reviewer.yml. | ||
* | ||
* For more information, see the Amazon CodeGuru Reviewer User Guide: | ||
* https://docs.aws.amazon.com/codeguru/latest/reviewer-ug/welcome.html. | ||
*/ | ||
public class FileSyntaxError { | ||
|
||
public void getObjectContentNoncompliant(AmazonS3 amazonS3Client, String bucketName, String key) throws IOException { | ||
final S3Object s3object = amazonS3Client.getObject(bucketName, key); | ||
// Noncompliant: the statement is incomplete and is missing ";" at the end. | ||
System.out.println(s3object.getObjectMetadata()) | ||
InputStream reportStream = s3object.getObjectContent(); | ||
IOUtils.toString(reportStream); | ||
} | ||
|
||
public void getObjectContentCompliant(AmazonS3 amazonS3Client, String bucketName, String key) throws IOException { | ||
final S3Object s3object = amazonS3Client.getObject(bucketName, key); | ||
System.out.println(s3object.getObjectMetadata()); | ||
InputStream reportStream = s3object.getObjectContent(); | ||
IOUtils.toString(reportStream); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
package com.shipmentEvents.handlers; | ||
|
||
import java.nio.charset.StandardCharsets; | ||
import java.time.Duration; | ||
import java.util.ArrayList; | ||
import java.util.HashMap; | ||
import java.util.Iterator; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Map.Entry; | ||
|
||
import javax.crypto.Cipher; | ||
import javax.crypto.SecretKey; | ||
import javax.crypto.spec.SecretKeySpec; | ||
|
||
import com.amazonaws.regions.Regions; | ||
import com.amazonaws.services.lambda.runtime.Context; | ||
import com.amazonaws.services.lambda.runtime.RequestHandler; | ||
import com.amazonaws.services.lambda.runtime.LambdaLogger; | ||
import com.amazonaws.services.lambda.runtime.events.ScheduledEvent; | ||
import com.amazonaws.services.s3.AmazonS3; | ||
import com.amazonaws.services.s3.AmazonS3ClientBuilder; | ||
import com.amazonaws.services.s3.model.DeleteObjectsRequest; | ||
import com.amazonaws.services.s3.model.DeleteObjectsRequest.KeyVersion; | ||
import com.amazonaws.services.s3.model.ObjectListing; | ||
import com.amazonaws.services.s3.model.S3ObjectSummary; | ||
import com.shipmentEvents.util.Constants; | ||
import com.shopify.ShopifySdk; | ||
import com.shopify.model.ShopifyShop; | ||
|
||
import org.apache.commons.lang3.tuple.MutablePair; | ||
import org.apache.commons.lang3.tuple.Pair; | ||
|
||
|
||
public class EventHandler implements RequestHandler<ScheduledEvent, String> { | ||
|
||
/** | ||
* Shipment events for a carrier are uploaded to separate S3 buckets based on the source of events. E.g., events originating from | ||
* the hand-held scanner are stored in a separate bucket than the ones from mobile App. The Lambda processes events from multiple | ||
* sources and updates the latest status of the package in a summary S3 bucket every 15 minutes. | ||
* | ||
* The events are stored in following format: | ||
* - Each status update is a file, where the name of the file is tracking number + random id. | ||
* - Each file has status and time-stamp as the first 2 lines respectively. | ||
* - The time at which the file is stored in S3 is not an indication of the time-stamp of the event. | ||
* - Once the status is marked as DELIVERED, we can stop tracking the package. | ||
* | ||
* A Sample files looks as below: | ||
* FILE-NAME-> '8787323232232332--55322798-dd29-4a04-97f4-93e18feed554' | ||
* >status:IN TRANSIT | ||
* >timestamp: 1573410202 | ||
* >Other fields like...tracking history and address | ||
*/ | ||
public String handleRequest(ScheduledEvent scheduledEvent, Context context) { | ||
|
||
final LambdaLogger logger = context.getLogger(); | ||
try { | ||
processShipmentUpdates(logger); | ||
return "SUCCESS"; | ||
} catch (final Exception ex) { | ||
logger.log(String.format("Failed to process shipment Updates in %s due to %s", scheduledEvent.getAccount(), ex.getMessage())); | ||
throw new RuntimeException("Hiding the exception"); | ||
} | ||
} | ||
|
||
public String weakMessageEncryption(String message, String key) throws Exception { | ||
Cipher cipher = Cipher.getInstance("RSA"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji. It looks like your code uses a cipher object with an insecure transformation. To make your code more secure, use one of the following algorithms with a built-in integrity check: |
||
SecretKey secretKey = new SecretKeySpec(key.getBytes(), "AES"); | ||
cipher.init(Cipher.ENCRYPT_MODE, secretKey); | ||
return new String(cipher.doFinal(message.getBytes()), StandardCharsets.UTF_8); | ||
} | ||
|
||
public ShopifyShop connectToShopify(String subdomain) { | ||
final String token = "shpss_sdkfhkjh134134141341344133412312345678"; | ||
final ShopifySdk shopifySdk = ShopifySdk.newBuilder() | ||
.withSubdomain(subdomain) | ||
.withAccessToken(token).build(); | ||
return shopifySdk.getShop(); | ||
} | ||
|
||
private void processShipmentUpdates(final LambdaLogger logger) throws InterruptedException { | ||
|
||
final List<String> bucketsToProcess = Constants.BUCKETS_TO_PROCESS; | ||
final Map<String, Pair<Long, String>> latestStatusForTrackingNumber = new HashMap<String, Pair<Long, String>>(); | ||
final Map<String, List<KeyVersion>> filesToDelete = new HashMap<String, List<DeleteObjectsRequest.KeyVersion>>(); | ||
for (final String bucketName : bucketsToProcess) { | ||
final List<KeyVersion> filesProcessed = processEventsInBucket(bucketName, logger, latestStatusForTrackingNumber); | ||
filesToDelete.put(bucketName, filesProcessed); | ||
} | ||
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
|
||
//Create a new file in the Constants.SUMMARY_BUCKET | ||
logger.log("Map of statuses -> " + latestStatusForTrackingNumber); | ||
String summaryUpdateName = Long.toString(System.currentTimeMillis()); | ||
|
||
EventHandler.getS3Client().putObject(Constants.SUMMARY_BUCKET, summaryUpdateName, latestStatusForTrackingNumber.toString()); | ||
|
||
long expirationTime = System.currentTimeMillis() + Duration.ofMinutes(1).toMillis(); | ||
while(System.currentTimeMillis() < expirationTime) { | ||
if (s3Client.doesObjectExist(Constants.SUMMARY_BUCKET, summaryUpdateName)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji. This code appears to be waiting for a resource before it runs. You could use the waiters feature to help improve efficiency. Consider using ObjectExists or ObjectNotExists. For more information, see https://aws.amazon.com/blogs/developer/waiters-in-the-aws-sdk-for-java/ |
||
break; | ||
} | ||
logger.log("waiting for file to be created " + summaryUpdateName); | ||
Thread.sleep(1000); | ||
} | ||
|
||
// Before we delete the shipment updates make sure the summary update file exists | ||
if (EventHandler.getS3Client().doesObjectExist(Constants.SUMMARY_BUCKET, summaryUpdateName)) { | ||
deleteProcessedFiles(filesToDelete); | ||
logger.log("All updates successfully processed"); | ||
} else { | ||
throw new RuntimeException("Failed to write summary status, will be retried in 15 minutes"); | ||
} | ||
|
||
} | ||
|
||
private List<KeyVersion> processEventsInBucket(String bucketName, LambdaLogger logger, Map<String, Pair<Long, String>> latestStatusForTrackingNumber) { | ||
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
logger.log("Processing Bucket: " + bucketName); | ||
|
||
ObjectListing files = s3Client.listObjects(bucketName); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji. This code might not produce accurate results if the operation returns paginated results instead of all results. Consider adding another call to check for additional results. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji. This code uses an outdated API. ListObjectsV2 is the revised List Objects API, and we recommend you use this revised API for new application developments. |
||
List<KeyVersion> filesProcessed = new ArrayList<DeleteObjectsRequest.KeyVersion>(); | ||
|
||
for (Iterator<?> iterator = files.getObjectSummaries().iterator(); iterator.hasNext(); ) { | ||
S3ObjectSummary summary = (S3ObjectSummary) iterator.next(); | ||
logger.log("Reading Object: " + summary.getKey()); | ||
|
||
String trackingNumber = summary.getKey().split("--")[0]; | ||
Pair<Long, String> lastKnownStatus = latestStatusForTrackingNumber.get(trackingNumber); | ||
|
||
// Check if this shipment has already been delivered, skip this file | ||
if (lastKnownStatus != null && "DELIVERED".equals(lastKnownStatus.getRight())) { | ||
continue; | ||
} | ||
|
||
String fileContents = s3Client.getObjectAsString(bucketName, summary.getKey()); | ||
|
||
if (!isValidFile(fileContents)) { | ||
logger.log(String.format("Skipping invalid file %s", summary.getKey())); | ||
continue; | ||
} | ||
|
||
if (!fileContents.contains("\n")) { | ||
|
||
} | ||
String[] lines = fileContents.split("\n"); | ||
String line1 = lines[0]; | ||
String line2 = lines[1]; | ||
|
||
String status = line1.split(":")[1]; | ||
Long timeStamp = Long.parseLong(line2.split(":")[1]); | ||
|
||
|
||
if (null == lastKnownStatus || lastKnownStatus.getLeft() < timeStamp) { | ||
lastKnownStatus = new MutablePair<Long, String>(timeStamp, status); | ||
latestStatusForTrackingNumber.put(trackingNumber, lastKnownStatus); | ||
} | ||
|
||
//Add to list of processed files | ||
filesProcessed.add(new KeyVersion(summary.getKey())); | ||
logger.log("logging Contents of the file" + fileContents); | ||
} | ||
return filesProcessed; | ||
} | ||
|
||
|
||
private void deleteProcessedFiles(Map<String, List<KeyVersion>> filesToDelete) { | ||
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
for (Entry<String, List<KeyVersion>> entry : filesToDelete.entrySet()) { | ||
final DeleteObjectsRequest deleteRequest = new DeleteObjectsRequest(entry.getKey()).withKeys(entry.getValue()).withQuiet(false); | ||
s3Client.deleteObjects(deleteRequest); | ||
} | ||
} | ||
|
||
|
||
private void deleteProcessedFiles(Map<String, List<KeyVersion>> filesToDelete) { | ||
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
for (Entry<String, List<KeyVersion>> entry : filesToDelete.entrySet()) { | ||
final DeleteObjectsRequest deleteRequest = new DeleteObjectsRequest(entry.getKey()).withKeys(entry.getValue()).withQuiet(false); | ||
s3Client.deleteObjects(deleteRequest); | ||
} | ||
} | ||
|
||
private boolean isValidFile(String fileContents) { | ||
if (!fileContents.contains("\n")) { | ||
return false; | ||
} | ||
String[] lines = fileContents.split("\n"); | ||
for (String l: lines) { | ||
if (!l.contains(":")) { | ||
return false; | ||
} | ||
} | ||
return true; | ||
} | ||
|
||
public static AmazonS3 getS3Client() { | ||
return AmazonS3ClientBuilder.standard().withRegion(Regions.DEFAULT_REGION).build(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji. This code is written so that the client cannot be reused across invocations of the Lambda function. |
||
} | ||
|
||
|
||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem: While re-throwing the caught exception with modifications , information about the caught exception is being lost, including information about the stack trace of the exception.
Fix: If the caught exception object does not contain sensitive information, consider passing it as the "rootCause" or inner exception parameter to the constructor of the new exception before throwing the new exception. (Note that not all exception constructors support inner exceptions. Use a wrapper exception that supports inner exceptions.)
Learn more
Suggested remediation:
Consider passing the exception object either as "rootCause" or as an inner exception parameter, to the constructor of the new exception before throwing new exception. This will help keep stack trace.