Skip to content

Commit

Permalink
Merge pull request #68 from usnistgov/fix/ODD-836-reponse-error
Browse files Browse the repository at this point in the history 
Improve exception handling and prevent file descriptor leaks.
  • Loading branch information
@RayPlante
RayPlante authored Jan 10, 2020
2 parents a706037 + 2ee0d6c commit cb10eb2
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 35 deletions.
73 changes: 43 additions & 30 deletions src/main/java/gov/nist/oar/distrib/datapackage/DefaultDataPackager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,42 +100,47 @@ public void getData(ZipOutputStream zout) throws IOException, DistributionExcept
HttpURLConnection con = null;
this.validateBundleRequest();

logger.info("Forming zip file from the the input fileurls");
logger.debug("Forming zip file from the the input fileurls");

for (int i = 0; i < inputfileList.length; i++) {
FileRequest jobject = inputfileList[i];
String filepath = jobject.getFilePath();
String downloadurl = jobject.getDownloadUrl();
if(this.validateUrl(downloadurl)) {
URLStatusLocation uLoc = listUrlsStatusSize.get(i);
if ((downloadurl.equalsIgnoreCase(uLoc.getRequestedURL())) && this.checkResponse(uLoc)) {
try {
URL obj = new URL(uLoc.getRequestedURL());
con = (HttpURLConnection) obj.openConnection();

int len;
byte[] buf = new byte[100000];
zout.putNextEntry(new ZipEntry(filepath));
InputStream fstream = con.getInputStream();
while ((len = fstream.read(buf)) != -1) {
zout.write(buf, 0, len);
}
zout.closeEntry();
fstream.close();
fileCount++;
} catch (IOException ie) {
bundlelogError.append("\n Exception in getting data for: " + filepath + " at " + downloadurl);
logger.error("There is an error reading this file at location: " + downloadurl + "Exception: "
+ ie.getMessage());
}
if (con != null)
con.disconnect();
}
URLStatusLocation uLoc = listUrlsStatusSize.get(i);
if ((downloadurl.equalsIgnoreCase(uLoc.getRequestedURL())) && this.checkResponse(uLoc)) {
InputStream fstream = null;
try {
URL obj = new URL(uLoc.getRequestedURL());
con = (HttpURLConnection) obj.openConnection();
fstream = con.getInputStream();
int len;
byte[] buf = new byte[100000];
zout.putNextEntry(new ZipEntry(filepath));
while ((len = fstream.read(buf)) != -1) {
zout.write(buf, 0, len);
}
zout.closeEntry();
fstream.close();
fileCount++;
} catch (IOException ie) {
bundlelogError.append("\n Exception in getting data for: " + filepath + " at " +
downloadurl+ ";\n this file might be corrupted.");
logger.error("There is an error reading this file at location: " + downloadurl + "Exception: "
+ ie.getMessage());
zout.closeEntry();
} finally {
if(fstream != null)
fstream.close();
if (con != null)
con.disconnect();
}
}
}
}

if (fileCount == 0) {
logger.info("The package does not contain any data. These errors :" + this.bundlelogError);
logger.warn("The package does not contain any data. These errors :" + this.bundlelogError);
throw new NoContentInPackageException("No data or files written in Bundle/Package.");
}
this.writeLog(zout);
Expand Down Expand Up @@ -184,7 +189,7 @@ private void dealWithErrors(URLStatusLocation uloc) {
String requestedUrl = uloc.getRequestedURL();
if (uloc.getStatus() >= 400 && uloc.getStatus() <= 500) {

logger.info(requestedUrl + " Error accessing this url: " + uloc.getStatus());
logger.error(requestedUrl + " Error accessing this url: " + uloc.getStatus());
this.bundlelogError.append("\n " + requestedUrl);
this.bundlelogError
.append(" There is an Error accessing this file, Server returned status with response code ");
Expand All @@ -208,8 +213,10 @@ private void dealWithErrors(URLStatusLocation uloc) {
* accessing requested URLs
*
* @param zout
* @throws IOException
*/
private void writeLog(ZipOutputStream zout) {
private void writeLog(ZipOutputStream zout) throws IOException {
InputStream nStream = null;
try {
String filename = "";
int l;
Expand All @@ -233,14 +240,20 @@ private void writeLog(ZipOutputStream zout) {
filename = "/PackagingSuccessful.txt";
}

InputStream nStream = new ByteArrayInputStream(bundleInfo.toString().getBytes());
nStream = new ByteArrayInputStream(bundleInfo.toString().getBytes());
zout.putNextEntry(new ZipEntry(filename));
while ((l = nStream.read(buf)) != -1) {
zout.write(buf, 0, l);
}
zout.closeEntry();
} catch (IOException ie) {
logger.info("Exception while creating Ziplogfile" + ie.getMessage());
logger.warn("Exception while creating Ziplogfile" + ie.getMessage());
if(zout != null)
zout.closeEntry();
}
finally {
if(nStream != null)
nStream.close();
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/gov/nist/oar/distrib/datapackage/DownloadBundlePlanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public BundleDownloadPlan getBundleDownloadPlan() throws DistributionException {
filePathUrls = new ArrayList<FileRequest>();
bundleFilePathUrls = new ArrayList<BundleRequest>();
messages = new ArrayList<String>();

logger.info("Creating bundle plan..");
try {
ObjectMapper mapper = new ObjectMapper();
String requestString = mapper.writeValueAsString(this.bundleRequest);
Expand All @@ -100,7 +100,8 @@ public BundleDownloadPlan getBundleDownloadPlan() throws DistributionException {
}
} catch (JsonProcessingException ex) {
// should not happen
throw new DistributionException("Trouble validating request: unable to conver to JSON: " +
logger.error("There is an issue validating request. unable to create valid JSON.");
throw new DistributionException("Trouble validating request: unable to convert to JSON: " +
ex.getMessage());
}

Expand All @@ -109,6 +110,7 @@ public BundleDownloadPlan getBundleDownloadPlan() throws DistributionException {
this.inputfileList = this.bundleRequest.getIncludeFiles();
ValidationHelper.removeDuplicates(this.inputfileList);
} catch (IOException ie) {
logger.error("Error while parsing request, not a valid JSON input."+ie.getMessage());
messages.add("Error while parsing the request. Check if it is valid JSON.");
this.status = "Error";
return makeBundlePlan();
Expand Down Expand Up @@ -149,6 +151,7 @@ public BundleDownloadPlan getBundleDownloadPlan() throws DistributionException {
* @throws IOException
*/
public void makeBundles(FileRequest jobject) {
logger.info("Make bundles planning.");
bundledFilesCount++;
URLStatusLocation uObj = ValidationHelper.getFileURLStatusSize(jobject.getDownloadUrl(), this.validdomains, this.allowedRedirects);
long individualFileSize = uObj.getLength();
Expand Down Expand Up @@ -214,6 +217,7 @@ private void updateMessagesAndStatus() {
* Object of BundleDownloadPlan after processing input request.
*/
public BundleDownloadPlan makeBundlePlan() {
logger.info("makeBundlePlan called.");
return new BundleDownloadPlan("_bundle", this.status,
bundleFilePathUrls.toArray(new BundleRequest[0]), messages.toArray(new String[0]),
notIncludedFiles.toArray(new NotIncludedFile[0]));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/gov/nist/oar/distrib/datapackage/ValidationHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private static URLStatusLocation checkURLStatusLocationSize(String url, int allo
conn.setReadTimeout(10000);
conn.setRequestMethod("HEAD");
int responseCode = conn.getResponseCode();
long length = conn.getContentLength();
long length = conn.getContentLengthLong();
allowedRedirects--;
String location = conn.getHeaderField("Location");

Expand Down
9 changes: 9 additions & 0 deletions src/main/java/gov/nist/oar/distrib/web/DataBundleAccessController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ public void getBundle(@Valid @RequestBody BundleRequest bundleRequest, @ApiIgnor
} catch (EmptyBundleRequestException ex) {
logger.warn("Empty bundle request sent");
throw new ServiceSyntaxException("Bundle Request has empty list of files and urls", ex);
} finally {
if(zout !=null) {
try {
zout.close();
} catch (IOException e) {
logger.error("Error while closing the output ZipOutputStream: "+e.getMessage());
throw new DistributionException("Zip output stream close error: " + e.getMessage(), e);
}
}
}

}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/gov/nist/oar/distrib/DownloadBundlePlannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public void getBundleDownloadPlan2Test()
assertEquals(bundlePlan.getStatus(), "warnings");
assertEquals(bundlePlan.getBundleNameFilePathUrl().length, 1);
assertEquals(bundlePlan.getBundleNameFilePathUrl()[0].getBundleName(), "testdownload-1.zip");
assertEquals(bundlePlan.getBundleNameFilePathUrl()[0].getIncludeFiles().length, 2);
assertEquals(bundlePlan.getBundleNameFilePathUrl()[0].getIncludeFiles().length, 1);

}

Expand Down Expand Up @@ -109,7 +109,7 @@ public void getBundleDownloadPlan3Test()
BundleRequest[] test = bundlePlan.getBundleNameFilePathUrl();
assertEquals(bundlePlan.getPostEachTo(), "_bundle");
assertEquals(bundlePlan.getStatus(), "warnings");
assertEquals(bundlePlan.getBundleNameFilePathUrl().length, 2);
assertEquals(bundlePlan.getBundleNameFilePathUrl().length, 1);
assertEquals(bundlePlan.getBundleNameFilePathUrl()[0].getBundleName(), "testdownload-1.zip");
assertEquals(bundlePlan.getBundleNameFilePathUrl()[0].getIncludeFiles().length, 1);

Expand Down

0 comments on commit cb10eb2

Please sign in to comment.