[webkit-reviews] review denied: [Bug 31395] Bugzilla should show images in git patches : [Attachment 43038] Patch v0

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has denied Shinichiro Hamaji
<hamaji at chromium.org>'s request for review:
Bug 31395: Bugzilla should show images in git patches
https://bugs.webkit.org/show_bug.cgi?id=31395

Attachment 43038: Patch v0
https://bugs.webkit.org/attachment.cgi?id=43038&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
Rather than writing a new decode-git-binary-patch script which uses
VCSUtils::decodeGitBinaryPatch(), I think the PrettyPatch.rb script should
instead use a git command to do its decoding work directly.  We can guarantee
that git will be installed on bugs.webkit.org, so we don't have to rely on a
Perl implementation of the git decoding algorithm to get the image data.

> diff --git a/BugsSite/PrettyPatch/PrettyPatch.rb
b/BugsSite/PrettyPatch/PrettyPatch.rb
>	       lines_with_contents = lines[startOfSections...lines.length]
>	       @sections = DiffSection.parse(lines_with_contents) unless
@binary
> -	       @image_url = "data:image/png;base64," + lines_with_contents.join
if @image
> +	       if @image
> +		   @image_url = "data:image/png;base64," +
lines_with_contents.join
> +	       elsif @git_image
> +		   begin
> +		       stdin, stdout, stderr =
*Open3.popen3(GIT_DECODE_PATCH_PATH)
> +		       stdin.puts(lines)
> +		       stdin.close
> +		       @image_urls = stdout.readlines
> +		       stdout.close
> +		       error = stderr.read
> +		       stderr.close
> +		       if error != ''
> +			   @image_error = "Failed to decode git binary
patch<pre>#{CGI.escapeHTML(error)}</pre>"
> +		       end
> +		   rescue
> +		       @image_error = "Exception raised during decoding git
binary patch: #{$!}"

The error message, $!, needs to be HTML-escaped as well.

r- to fix the HTML-escaping issue noted and to use a git command directly to
decode the binary patch.