Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(177)

Issue 1722047: Fix NPE in BasicImageRewriter

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 9 months ago by jtarrio
Modified:
15 years, 9 months ago
Reviewers:
johnfargo, shindig.remailer
Base URL:
http://svn.apache.org/repos/asf/shindig/trunk/
Visibility:
Public.

Description

Fixes a NPE in the BasicImageRewriter when the no_expand query parameter is specified but one of resize_w or resize_h is not specified.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java View 1 chunk +2 lines, -2 lines 1 comment Download

Messages

Total messages: 1
johnfargo
15 years, 9 months ago (2010-06-28 21:14:56 UTC) #1
One thought, plus could I get a quick test for this?

http://codereview.appspot.com/1722047/diff/1/2
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java
(right):

http://codereview.appspot.com/1722047/diff/1/2#newcode149
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java:149:
(requestedWidth == null || imageInfo.getWidth() <= requestedWidth)) {
This is clearly better than status quo, and I'll submit it.

Even so, it would be nice to support no_expand=1 with one or the other of
resize_w/resize_h (providing a scaled bounding-box). Doing so mandates moving
the logic @175 et al higher in the class, to compute whichever is missing from
whichever is present. Thoughts?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b