Skip to content

Commit 1195ee5

Browse files
authored
BXC-4801 - Fix PDF viewer errors (#1866)
* When setting up pdf viewer property for full record responses, ensure that the user has permission to view the PDF file. If a pdf view is requested for a non-pdf file, then clear the viewer settings. * Add complete testing of viewer properties * Refactor getViewerProperties to try to reduce complexity * Use constant * Update visibility
1 parent 09662bd commit 1195ee5

File tree

3 files changed

+426
-36
lines changed

3 files changed

+426
-36
lines changed

Diff for: web-access-app/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,13 @@
148148
<artifactId>persistence</artifactId>
149149
<type>test-jar</type>
150150
</dependency>
151+
<!-- Used by spring-test for mvc assertions-->
152+
<dependency>
153+
<groupId>org.hamcrest</groupId>
154+
<artifactId>hamcrest</artifactId>
155+
<version>2.2</version>
156+
<scope>test</scope>
157+
</dependency>
151158

152159
<!-- CDR -->
153160
<dependency>

Diff for: web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FullRecordController.java

+60-36
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,12 @@
7474
@RequestMapping("/api/record")
7575
public class FullRecordController extends AbstractErrorHandlingSearchController {
7676
private static final Logger LOG = LoggerFactory.getLogger(FullRecordController.class);
77-
private final String VIEWER_PID = "viewerPid";
78-
private final String VIEWER_TYPE = "viewerType";
79-
private final String STREAMING_URL = "streamingUrl";
80-
private final String STREAMING_TYPE = "streamingType";
81-
private final String APPLICATION_X_PDF_VALUE = "application/x-pdf";
77+
protected static final String VIEWER_PID = "viewerPid";
78+
protected static final String VIEWER_TYPE = "viewerType";
79+
protected static final String STREAMING_URL = "streamingUrl";
80+
protected static final String STREAMING_TYPE = "streamingType";
81+
private static final String APPLICATION_X_PDF_VALUE = "application/x-pdf";
82+
protected static final String AV_MIMETYPE_REGEX = "(" + AUDIO_MIMETYPE_REGEX + ")|(" + VIDEO_MIMETYPE_REGEX + ")";
8283

8384
@Autowired
8485
private AccessControlService aclService;
@@ -99,7 +100,7 @@ public class FullRecordController extends AbstractErrorHandlingSearchController
99100
@Autowired
100101
private ObjectAclFactory objectAclFactory;
101102

102-
@Autowired(required = true)
103+
@Autowired
103104
private XSLViewResolver xslViewResolver;
104105
@Autowired
105106
private RepositoryObjectLoader repositoryObjectLoader;
@@ -272,65 +273,88 @@ public String handlePdfViewerRequest(@PathVariable("pid") String pidString, Mode
272273
SimpleIdRequest idRequest = new SimpleIdRequest(pid, principals);
273274
ContentObjectRecord briefObject = queryLayer.getObjectById(idRequest);
274275

275-
String viewerPid = null;
276+
String viewerPid;
276277
if (ResourceType.Work.nameEquals(briefObject.getResourceType())) {
277-
viewerPid = accessCopiesService.getFirstMatchingChild(briefObject,
278-
Arrays.asList(APPLICATION_PDF_VALUE, APPLICATION_X_PDF_VALUE), principals).getId();
278+
var child = accessCopiesService.getFirstMatchingChild(briefObject,
279+
Arrays.asList(APPLICATION_PDF_VALUE, APPLICATION_X_PDF_VALUE), principals);
280+
if (child == null) {
281+
throw new NotFoundException("Cannot find child PDF for " + pidString);
282+
}
283+
viewerPid = child.getId();
279284
} else {
280-
accessCopiesService.getDatastreamPid(briefObject, principals, PDF_MIMETYPE_REGEX);
285+
viewerPid = accessCopiesService.getDatastreamPid(briefObject, principals, PDF_MIMETYPE_REGEX);
286+
if (viewerPid == null) {
287+
throw new IllegalArgumentException("Resource is not a PDF: " + pidString);
288+
}
281289
}
282290

283-
model.addAttribute("viewerPid", viewerPid);
291+
model.addAttribute(VIEWER_PID, viewerPid);
284292
model.addAttribute("briefObject", briefObject);
285293
model.addAttribute("template", "ajax");
286294

287295
return "fullRecord/pdfViewer";
288296
}
289297

290298
private Map<String, Object> getViewerProperties(ContentObjectRecord briefObject, AccessGroupSet principals) {
291-
String viewerType = null;
292-
String viewerPid = null;
293-
String streamingUrl = null;
294-
String streamingType = null;
299+
String viewerType;
300+
String viewerPid ;
295301
ContentObjectRecord workStreamingContent = null;
296302

297303
boolean imageViewerNeeded = accessCopiesService.hasViewableFiles(briefObject, principals);
304+
if (imageViewerNeeded) {
305+
return makeViewerProperties("clover", null, null, null);
306+
}
298307

299-
if (!imageViewerNeeded) {
308+
boolean hasStreamingContent = briefObject.getContentStatus().contains(FacetConstants.HAS_STREAMING);
309+
if (!hasStreamingContent) {
300310
workStreamingContent = accessCopiesService.getFirstStreamingChild(briefObject, principals);
301311
}
302312

303-
if (imageViewerNeeded) {
313+
if (hasStreamingContent || workStreamingContent != null) {
314+
return makeStreamingProperties((workStreamingContent != null) ? workStreamingContent : briefObject);
315+
}
316+
317+
viewerPid = accessCopiesService.getDatastreamPid(briefObject, principals, AV_MIMETYPE_REGEX);
318+
319+
if (viewerPid != null) {
304320
viewerType = "clover";
305-
} else if (briefObject.getContentStatus().contains(FacetConstants.HAS_STREAMING) || workStreamingContent != null) {
306-
viewerType = "streaming";
307-
streamingUrl = (workStreamingContent != null) ? workStreamingContent.getStreamingUrl() :
308-
briefObject.getStreamingUrl();
309-
streamingType = (workStreamingContent != null) ? workStreamingContent.getStreamingType() :
310-
briefObject.getStreamingType();
311321
} else {
312-
viewerPid = accessCopiesService.getDatastreamPid(briefObject, principals,
313-
"(" + AUDIO_MIMETYPE_REGEX + ")|(" + VIDEO_MIMETYPE_REGEX + ")");
314-
315-
if (viewerPid != null) {
316-
viewerType = "clover";
317-
} else {
318-
viewerPid = accessCopiesService.getDatastreamPid(briefObject, principals, PDF_MIMETYPE_REGEX);
319-
if (viewerPid != null) {
320-
viewerType = "pdf";
321-
}
322-
}
322+
viewerPid = accessCopiesService.getDatastreamPid(briefObject, principals, PDF_MIMETYPE_REGEX);
323+
viewerType = viewerPid != null ? "pdf" : null;
324+
}
325+
326+
// When the viewer object is not the current object, check if the user has access to view the viewer object
327+
if (noAccessToViewChildObject(viewerPid, briefObject, principals)) {
328+
viewerPid = null;
329+
viewerType = null;
323330
}
324331

332+
return makeViewerProperties(viewerType, viewerPid, null, null);
333+
}
334+
335+
private Map<String, Object> makeStreamingProperties(ContentObjectRecord briefObject) {
336+
return makeViewerProperties("streaming",
337+
null,
338+
briefObject.getStreamingUrl(),
339+
briefObject.getStreamingType());
340+
}
341+
342+
private Map<String, Object> makeViewerProperties(String viewType, String viewerPid,
343+
String streamingUrl, String streamingType) {
325344
var viewerProperties = new HashMap<String, Object>();
326-
viewerProperties.put(VIEWER_TYPE, viewerType);
345+
viewerProperties.put(VIEWER_TYPE, viewType);
327346
viewerProperties.put(VIEWER_PID, viewerPid);
328347
viewerProperties.put(STREAMING_URL, streamingUrl);
329348
viewerProperties.put(STREAMING_TYPE, streamingType);
330-
331349
return viewerProperties;
332350
}
333351

352+
private boolean noAccessToViewChildObject(String viewerPid, ContentObjectRecord briefObject,
353+
AccessGroupSet principals) {
354+
return viewerPid != null && !briefObject.getId().equals(viewerPid)
355+
&& !aclService.hasAccess(PIDs.get(viewerPid), principals, Permission.viewOriginal);
356+
}
357+
334358
/**
335359
* Get list of digital exhibits associated with an object
336360
* @param briefObject

0 commit comments

Comments
 (0)