[Webkit-unassigned] [Bug 31395] Bugzilla should show images in git patches

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 4 17:41:37 PST 2009


https://bugs.webkit.org/show_bug.cgi?id=31395


David Kilzer (ddkilzer) <ddkilzer at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #44312|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #18 from David Kilzer (ddkilzer) <ddkilzer at webkit.org>  2009-12-04 17:41:37 PST ---
(From update of attachment 44312)
> +        def self.extract_contents_from_git_binary_chunk(encoded_chunk, git_index)
> +            # We use Tempfile we need a unique file among processes.
> +            tempfile = Tempfile.new("PrettyPatch")
> +            # We need a filename which doesn't exist to apply a patch
> +            # which creates a new file. Append a suffix so filename
> +            # doesn't exist.
> +            filename = File.basename(tempfile.path) + '.bin'

Prepending ".bin" to the unique filename may (potentially) lead to a symlink
attack because there is a short time between the patch file being written and
the patch being applied.  It would probably be best to generate the "bin"
filename as well.

> +            patch = FileDiff.git_new_file_binary_patch(filename, encoded_chunk, git_index)
> +
> +            # Apply the git binary patch using git-apply.
> +            Dir.chdir(File.dirname(tempfile.path)) do
> +                stdin, stdout, stderr = *Open3.popen3(GIT_PATH + " apply")
> +                begin
> +                    stdin.puts(patch)
> +                    stdin.close
> +
> +                    error = stderr.read
> +                    raise error if error != ""
> +
> +                    contents = File.read(filename)
> +                ensure
> +                    stdin.close unless stdin.closed?
> +                    stdout.close
> +                    stderr.close
> +                    File.unlink(filename) if File.exists?(filename)
> +                end
> +
> +                return nil if contents.empty?
> +                return "data:image/png;base64," + [contents].pack("m")
> +            end
> +        end
>      end

I don't see tempfile being unlinked anywhere, or is that implicit when its
object is deleted?  If not, it needs to be removed as well.

r=me if you fix the above two issues.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list