Skip to content

Commit 87f58e1

Browse files
sharonluongsharonluong
authored
Merge pull request #1794 from UNC-Libraries/bxc-4709
BXC-4709 - Handle range end >= file size
2 parents f66fb33 + 4b5c529 commit 87f58e1

File tree

4 files changed

+203
-12
lines changed

4 files changed

+203
-12
lines changed

Diff for: ‎web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerIT.java

+22-1
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,27 @@ public void testGetDatastreamWithRange() throws Exception {
130130
assertEquals("inline; filename=\"file.txt\"", response.getHeader(CONTENT_DISPOSITION));
131131
}
132132

133+
@Test
134+
public void testGetDatastreamWithRangeEndingSameAsSize() throws Exception {
135+
PID filePid = makePid();
136+
137+
FileObject fileObj = repositoryObjectFactory.createFileObject(filePid, null);
138+
fileObj.addOriginalFile(makeContentUri(originalPid(fileObj), BINARY_CONTENT), "file.txt", "text/plain", null, null);
139+
140+
MvcResult result = mvc.perform(get("/content/" + filePid.getId())
141+
.header(RANGE,"bytes=0-14"))
142+
.andExpect(status().is2xxSuccessful())
143+
.andReturn();
144+
145+
// Verify content was retrieved
146+
MockHttpServletResponse response = result.getResponse();
147+
assertEquals(BINARY_CONTENT, response.getContentAsString());
148+
149+
assertEquals(BINARY_CONTENT.length(), response.getContentAsString().length());
150+
assertEquals("text/plain", response.getContentType());
151+
assertEquals("inline; filename=\"file.txt\"", response.getHeader(CONTENT_DISPOSITION));
152+
}
153+
133154
@Test
134155
public void testGetDatastreamDownload() throws Exception {
135156
PID filePid = makePid();
@@ -284,7 +305,7 @@ public void testRangeExceedsFileLength() throws Exception {
284305
fileObj.addOriginalFile(makeContentUri(originalPid(fileObj), BINARY_CONTENT), null, "text/plain", null, null);
285306

286307
mvc.perform(get("/indexablecontent/" + filePid.getId())
287-
.header(RANGE,"bytes=0-900000"))
308+
.header(RANGE,"bytes=900000-900000"))
288309
.andExpect(status().isRequestedRangeNotSatisfiable())
289310
.andReturn();
290311
}

Diff for: ‎web-common/src/main/java/edu/unc/lib/boxc/web/common/services/FedoraContentService.java

+22
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ public void streamData(PID pid, String dsName, boolean asAttachment,
9696
binObj = repositoryObjectLoader.getBinaryObject(dsPid);
9797
}
9898

99+
// Make sure the range is valid or will produce a reasonable response from fedora
100+
range = correctRangeValue(range, binObj);
101+
99102
response.setHeader(CONTENT_TYPE, binObj.getMimetype());
100103
String binaryName = binObj.getFilename();
101104
String filename = binaryName == null ? pid.getId() : binaryName;
@@ -116,6 +119,25 @@ public void streamData(PID pid, String dsName, boolean asAttachment,
116119
}
117120
}
118121

122+
private String correctRangeValue(String range, BinaryObject binObj) {
123+
if (range == null) {
124+
return null;
125+
}
126+
var rangeParts = range.split("-");
127+
if (rangeParts.length == 2) {
128+
var endingRange = rangeParts[1];
129+
try {
130+
var endingValue = Long.parseLong(endingRange);
131+
if (endingValue >= binObj.getFilesize()) {
132+
return rangeParts[0] + "-";
133+
}
134+
} catch (NumberFormatException e) {
135+
LOG.debug("Invalid range ending provided: {}", e.getMessage());
136+
}
137+
}
138+
return range;
139+
}
140+
119141
public void streamEventLog(PID pid, AccessGroupSet principals, boolean asAttachment,
120142
HttpServletResponse response) throws IOException {
121143

Diff for: ‎web-common/src/test/java/edu/unc/lib/boxc/web/common/services/FedoraContentServiceTest.java

+158-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
package edu.unc.lib.boxc.web.common.services;
22

33
import edu.unc.lib.boxc.auth.api.services.AccessControlService;
4+
import edu.unc.lib.boxc.auth.fcrepo.models.AccessGroupSetImpl;
5+
import edu.unc.lib.boxc.fcrepo.exceptions.RangeNotSatisfiableException;
6+
import edu.unc.lib.boxc.model.api.DatastreamType;
7+
import edu.unc.lib.boxc.model.api.event.PremisLog;
48
import edu.unc.lib.boxc.model.api.exceptions.NotFoundException;
9+
import edu.unc.lib.boxc.model.api.ids.PID;
510
import edu.unc.lib.boxc.model.api.objects.BinaryObject;
611
import edu.unc.lib.boxc.model.api.objects.FileObject;
712
import edu.unc.lib.boxc.model.api.objects.RepositoryObjectLoader;
13+
import edu.unc.lib.boxc.model.api.rdf.DcElements;
14+
import edu.unc.lib.boxc.model.fcrepo.ids.DatastreamPids;
15+
import org.apache.jena.rdf.model.ModelFactory;
816
import org.fcrepo.client.FcrepoClient;
917
import org.fcrepo.client.FcrepoOperationFailedException;
1018
import org.fcrepo.client.FcrepoResponse;
@@ -19,19 +27,22 @@
1927
import javax.servlet.http.HttpServletResponse;
2028

2129
import java.io.ByteArrayInputStream;
22-
import java.io.ByteArrayOutputStream;
2330
import java.io.IOException;
2431
import java.nio.charset.StandardCharsets;
2532

2633
import static edu.unc.lib.boxc.model.api.DatastreamType.ORIGINAL_FILE;
2734
import static edu.unc.lib.boxc.model.fcrepo.test.TestHelper.makePid;
2835
import static org.apache.http.HttpHeaders.CONTENT_LENGTH;
29-
import static org.junit.jupiter.api.Assertions.assertEquals;
36+
import static org.apache.http.HttpHeaders.CONTENT_RANGE;
37+
import static org.apache.http.HttpHeaders.CONTENT_TYPE;
38+
import static org.junit.jupiter.api.Assertions.assertThrows;
3039
import static org.mockito.ArgumentMatchers.any;
3140
import static org.mockito.ArgumentMatchers.eq;
41+
import static org.mockito.Mockito.mock;
3242
import static org.mockito.Mockito.verify;
3343
import static org.mockito.Mockito.when;
3444
import static org.mockito.MockitoAnnotations.openMocks;
45+
import static org.springframework.http.HttpHeaders.CONTENT_DISPOSITION;
3546

3647
public class FedoraContentServiceTest {
3748
private AutoCloseable closeable;
@@ -54,14 +65,18 @@ public class FedoraContentServiceTest {
5465
private FcrepoResponse fcrepoResponse;
5566
@Mock
5667
private ServletOutputStream outputStream;
68+
@Mock
69+
private PremisLog premisLog;
5770

5871
@BeforeEach
59-
public void setup() {
72+
public void setup() throws Exception {
6073
closeable = openMocks(this);
6174
fedoraContentService = new FedoraContentService();
6275
fedoraContentService.setClient(fcrepoClient);
6376
fedoraContentService.setAccessControlService(accessControlService);
6477
fedoraContentService.setRepositoryObjectLoader(repositoryObjectLoader);
78+
when(response.getOutputStream()).thenReturn(outputStream);
79+
when(fileObject.getPremisLog()).thenReturn(premisLog);
6580
}
6681

6782
@AfterEach
@@ -90,18 +105,151 @@ public void streamDataWithExternalDatastream() {
90105
@Test
91106
public void streamDataSuccess() throws IOException, FcrepoOperationFailedException {
92107
var pid = makePid();
93-
when(repositoryObjectLoader.getFileObject(eq(pid))).thenReturn(fileObject);
94-
when(fileObject.getOriginalFile()).thenReturn(binaryObject);
95-
when(binaryObject.getFilename()).thenReturn("Best Name");
96-
when(binaryObject.getPid()).thenReturn(pid);
97-
when(fcrepoClient.get(any())).thenReturn(builder);
98-
when(builder.perform()).thenReturn(fcrepoResponse);
108+
mockWithOriginalFile(pid);
99109
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("image".getBytes(StandardCharsets.UTF_8)));
100-
when(response.getOutputStream()).thenReturn(outputStream);
101110
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("5");
102111

103112
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), false, response, null);
104113

105114
verify(response).setHeader(CONTENT_LENGTH, "5");
115+
verify(response).setHeader(CONTENT_DISPOSITION, "inline; filename=\"Best Name\"");
116+
}
117+
118+
@Test
119+
public void streamDataSuccessAsAttachment() throws IOException, FcrepoOperationFailedException {
120+
var pid = makePid();
121+
mockWithOriginalFile(pid);
122+
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("image".getBytes(StandardCharsets.UTF_8)));
123+
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("5");
124+
125+
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), true, response, null);
126+
127+
verify(response).setHeader(CONTENT_LENGTH, "5");
128+
verify(response).setHeader(CONTENT_DISPOSITION, "attachment; filename=\"Best Name\"");
129+
}
130+
131+
@Test
132+
public void streamDataSuccessOtherDatastream() throws IOException, FcrepoOperationFailedException {
133+
var pid = makePid();
134+
var modsPid = DatastreamPids.getMdDescriptivePid(pid);
135+
mockWithOriginalFile(pid);
136+
var modsBinary = mock(BinaryObject.class);
137+
when(modsBinary.getFilename()).thenReturn("mods.xml");
138+
when(modsBinary.getPid()).thenReturn(modsPid);
139+
when(modsBinary.getFilesize()).thenReturn(4L);
140+
when(repositoryObjectLoader.getBinaryObject(eq(modsPid))).thenReturn(modsBinary);
141+
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("desc".getBytes(StandardCharsets.UTF_8)));
142+
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("4");
143+
144+
fedoraContentService.streamData(pid, DatastreamType.MD_DESCRIPTIVE.getId(), false, response, null);
145+
146+
verify(response).setHeader(CONTENT_LENGTH, "4");
147+
verify(response).setHeader(CONTENT_DISPOSITION, "inline; filename=\"mods.xml\"");
148+
}
149+
150+
@Test
151+
public void streamDataWithValidRange() throws IOException, FcrepoOperationFailedException {
152+
var pid = makePid();
153+
mockWithOriginalFile(pid);
154+
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("imag".getBytes(StandardCharsets.UTF_8)));
155+
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("4");
156+
var contentRange = "bytes 0-3/5";
157+
when(fcrepoResponse.getHeaderValue(CONTENT_RANGE)).thenReturn(contentRange);
158+
159+
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), false, response, "bytes=0-3");
160+
161+
verify(response).setHeader(CONTENT_LENGTH, "4");
162+
verify(response).setHeader(CONTENT_RANGE, contentRange);
163+
verify(builder).addHeader("Range", "bytes=0-3");
164+
}
165+
166+
@Test
167+
public void streamDataWithEndRangeSameAsSize() throws IOException, FcrepoOperationFailedException {
168+
var pid = makePid();
169+
mockWithOriginalFile(pid);
170+
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("image".getBytes(StandardCharsets.UTF_8)));
171+
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("5");
172+
var contentRange = "bytes 0-4/5";
173+
when(fcrepoResponse.getHeaderValue(CONTENT_RANGE)).thenReturn(contentRange);
174+
175+
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), false, response, "bytes=0-5");
176+
177+
verify(response).setHeader(CONTENT_LENGTH, "5");
178+
verify(response).setHeader(CONTENT_RANGE, contentRange);
179+
verify(builder).addHeader("Range", "bytes=0-");
180+
}
181+
182+
@Test
183+
public void streamDataWithEndRangeGreaterThanSize() throws IOException, FcrepoOperationFailedException {
184+
var pid = makePid();
185+
mockWithOriginalFile(pid);
186+
when(fcrepoResponse.getBody()).thenReturn(new ByteArrayInputStream("mage".getBytes(StandardCharsets.UTF_8)));
187+
when(fcrepoResponse.getHeaderValue(CONTENT_LENGTH)).thenReturn("4");
188+
var contentRange = "bytes 1-4/5";
189+
when(fcrepoResponse.getHeaderValue(CONTENT_RANGE)).thenReturn(contentRange);
190+
191+
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), false, response, "bytes=1-8");
192+
193+
verify(response).setHeader(CONTENT_LENGTH, "4");
194+
verify(response).setHeader(CONTENT_RANGE, contentRange);
195+
verify(builder).addHeader("Range", "bytes=1-");
196+
}
197+
198+
@Test
199+
public void streamDataWithEndRangeInvalid() throws IOException, FcrepoOperationFailedException {
200+
var pid = makePid();
201+
mockWithOriginalFile(pid);
202+
when(builder.perform()).thenThrow(new FcrepoOperationFailedException(null, 416, "Bad Range"));
203+
when(fcrepoResponse.getStatusCode()).thenReturn(416);
204+
205+
assertThrows(RangeNotSatisfiableException.class, () -> {
206+
fedoraContentService.streamData(pid, ORIGINAL_FILE.getId(), false, response, "bytes=1-bad");
207+
});
208+
}
209+
210+
@Test
211+
public void streamEventLogTest() throws IOException, FcrepoOperationFailedException {
212+
var pid = makePid();
213+
214+
when(repositoryObjectLoader.getRepositoryObject(eq(pid))).thenReturn(fileObject);
215+
var model = ModelFactory.createDefaultModel();
216+
when(premisLog.getEventsModel()).thenReturn(model);
217+
var logResource = model.getResource(pid.getRepositoryPath());
218+
logResource.addProperty(DcElements.title, "test title");
219+
220+
var principals = new AccessGroupSetImpl("group");
221+
222+
fedoraContentService.streamEventLog(pid, principals, false, response);
223+
224+
verify(response).setHeader(CONTENT_TYPE, "text/turtle");
225+
verify(response).setHeader(CONTENT_DISPOSITION, "inline; filename=\"" + pid.getId() + "_event_log.ttl\"");
226+
}
227+
228+
@Test
229+
public void streamEventLogAttachmentTest() throws IOException, FcrepoOperationFailedException {
230+
var pid = makePid();
231+
when(repositoryObjectLoader.getRepositoryObject(eq(pid))).thenReturn(fileObject);
232+
233+
var model = ModelFactory.createDefaultModel();
234+
when(premisLog.getEventsModel()).thenReturn(model);
235+
var logResource = model.getResource(pid.getRepositoryPath());
236+
logResource.addProperty(DcElements.title, "test title");
237+
238+
var principals = new AccessGroupSetImpl("group");
239+
240+
fedoraContentService.streamEventLog(pid, principals, true, response);
241+
242+
verify(response).setHeader(CONTENT_TYPE, "text/turtle");
243+
verify(response).setHeader(CONTENT_DISPOSITION, "attachment; filename=\"" + pid.getId() + "_event_log.ttl\"");
244+
}
245+
246+
private void mockWithOriginalFile(PID pid) throws FcrepoOperationFailedException {
247+
when(repositoryObjectLoader.getFileObject(eq(pid))).thenReturn(fileObject);
248+
when(fileObject.getOriginalFile()).thenReturn(binaryObject);
249+
when(binaryObject.getFilename()).thenReturn("Best Name");
250+
when(binaryObject.getPid()).thenReturn(pid);
251+
when(binaryObject.getFilesize()).thenReturn(5L);
252+
when(fcrepoClient.get(any())).thenReturn(builder);
253+
when(builder.perform()).thenReturn(fcrepoResponse);
106254
}
107255
}

Diff for: ‎web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/DatastreamRestControllerIT.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ public void testRangeExceedsFileLength() throws Exception {
407407
fileObj.addOriginalFile(contentPath.toUri(), "file.txt", "text/plain", null, null);
408408

409409
mvc.perform(get("/file/" + filePid.getId())
410-
.header(RANGE,"bytes=0-900000"))
410+
.header(RANGE,"bytes=900000-900001"))
411411
.andExpect(status().isRequestedRangeNotSatisfiable())
412412
.andReturn();
413413
}

0 commit comments

Comments
 (0)