From 32a2f33dc80818b497e20b73a8551094c38106a8 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 10 Oct 2013 00:36:45 -0500 Subject: [PATCH] Rework some GZipWriter construction logic to fix #1088 * Fix open->newInstance sequence so initialize has the right arity * Process level and strategy on the way in * Pass level to jzlib * Peel off opt hash correctly Still to do: * Pass strategy to jzlib * Match MRI call args for deflateInit2 * Other tweakery to match MRI logic. See also #1108 --- .../jruby/ext/zlib/JZlibRubyGzipWriter.java | 60 ++++++++++++------- .../java/org/jruby/ext/zlib/RubyGzipFile.java | 12 ---- .../java/org/jruby/ext/zlib/RubyZlib.java | 6 ++ 3 files changed, 43 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/jruby/ext/zlib/JZlibRubyGzipWriter.java b/core/src/main/java/org/jruby/ext/zlib/JZlibRubyGzipWriter.java index a6e63782bc4..e7faff3d6bb 100644 --- a/core/src/main/java/org/jruby/ext/zlib/JZlibRubyGzipWriter.java +++ b/core/src/main/java/org/jruby/ext/zlib/JZlibRubyGzipWriter.java @@ -2,6 +2,8 @@ */ package org.jruby.ext.zlib; +import com.jcraft.jzlib.Deflater; +import com.jcraft.jzlib.JZlib; import java.io.IOException; import org.jcodings.specific.ASCIIEncoding; import org.joda.time.DateTime; @@ -29,6 +31,7 @@ import org.jruby.util.ByteList; import org.jruby.util.encoding.Transcoder; import org.jruby.util.IOOutputStream; +import org.jruby.util.TypeConverter; /** * @@ -54,8 +57,8 @@ public static JZlibRubyGzipWriter newInstance(IRubyObject recv, IRubyObject[] ar @JRubyMethod(name = "open", required = 1, optional = 2, meta = true, compat = RUBY1_8) public static IRubyObject open18(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) { Ruby runtime = recv.getRuntime(); - IRubyObject io = Helpers.invoke(context, runtime.getFile(), "open", args[0], runtime.newString("wb")); - JZlibRubyGzipWriter gzio = newInstance(recv, argsWithIo(io, args), block); + args[0] = Helpers.invoke(context, runtime.getFile(), "open", args[0], runtime.newString("wb")); + JZlibRubyGzipWriter gzio = newInstance(recv, args, block); return RubyGzipFile.wrapBlock(context, gzio, block); } @@ -63,8 +66,8 @@ public static IRubyObject open18(ThreadContext context, IRubyObject recv, IRubyO @JRubyMethod(name = "open", required = 1, optional = 3, meta = true, compat = RUBY1_9) public static IRubyObject open19(ThreadContext context, IRubyObject recv, IRubyObject[] args, Block block) { Ruby runtime = recv.getRuntime(); - IRubyObject io = Helpers.invoke(context, runtime.getFile(), "open", args[0], runtime.newString("wb")); - JZlibRubyGzipWriter gzio = newInstance(recv, argsWithIo(io, args), block); + args[0] = Helpers.invoke(context, runtime.getFile(), "open", args[0], runtime.newString("wb")); + JZlibRubyGzipWriter gzio = newInstance(recv, args, block); return RubyGzipFile.wrapBlock(context, gzio, block); } @@ -76,17 +79,21 @@ public JZlibRubyGzipWriter(Ruby runtime, RubyClass type) { @JRubyMethod(name = "initialize", required = 1, rest = true, visibility = PRIVATE, compat = RUBY1_8) public IRubyObject initialize(IRubyObject[] args) { - // args: recv, path, opts = {} - if (args.length > 2) { - checkLevel(getRuntime(), RubyNumeric.fix2int(args[2])); - } - return initializeCommon(args[0]); + Ruby runtime = getRuntime(); + + level = args.length < 2 ? JZlib.Z_DEFAULT_COMPRESSION : RubyZlib.FIXNUMARG(args[1], JZlib.Z_DEFAULT_COMPRESSION); + checkLevel(runtime, level); + // unused; could not figure out how to get JZlib to take these right + int strategy = args.length < 3 ? JZlib.Z_DEFAULT_STRATEGY : RubyZlib.FIXNUMARG(args[2], JZlib.Z_DEFAULT_STRATEGY); + return initializeCommon(args[0], level); } - private IRubyObject initializeCommon(IRubyObject stream) { + private IRubyObject initializeCommon(IRubyObject stream, int level) { realIo = (RubyObject) stream; try { - io = new com.jcraft.jzlib.GZIPOutputStream(new IOOutputStream(realIo, false, false), 512, false); + // the 15+16 here is copied from a Deflater default constructor + Deflater deflater = new Deflater(level, 15+16, false); + io = new com.jcraft.jzlib.GZIPOutputStream(new IOOutputStream(realIo, false, false), deflater, 512, false); return this; } catch (IOException ioe) { throw getRuntime().newIOErrorFromException(ioe); @@ -95,36 +102,43 @@ private IRubyObject initializeCommon(IRubyObject stream) { @JRubyMethod(name = "initialize", rest = true, visibility = PRIVATE, compat = RUBY1_9) public IRubyObject initialize19(ThreadContext context, IRubyObject[] args, Block unused) { - // args: recv, path, level = nil, strategy = nil, opts = {} - IRubyObject obj = initializeCommon(args[0]); + Ruby runtime = context.getRuntime(); IRubyObject opt = context.nil; - if (args.length > 3) { - opt = args[args.length - 1]; + + int argc = args.length; + if (argc > 1) { + opt = TypeConverter.checkHashType(runtime, opt); + if (!opt.isNil()) argc--; } - ecopts(context, opt); + level = argc < 2 ? JZlib.Z_DEFAULT_COMPRESSION : RubyZlib.FIXNUMARG(args[1], JZlib.Z_DEFAULT_COMPRESSION); + checkLevel(runtime, level); + // unused; could not figure out how to get JZlib to take these right + int strategy = argc < 3 ? JZlib.Z_DEFAULT_STRATEGY : RubyZlib.FIXNUMARG(args[2], JZlib.Z_DEFAULT_STRATEGY); - if (args.length > 2) { - checkLevel(getRuntime(), RubyNumeric.fix2int(args[2])); - } + IRubyObject obj = initializeCommon(args[0], level); + ecopts(context, opt); + + // FIXME: don't singletonize! if (realIo.respondsTo("path")) { - obj.getSingletonClass().addMethod("path", new JavaMethod.JavaMethodZero(obj.getSingletonClass(), Visibility.PUBLIC) { + getSingletonClass().addMethod("path", new JavaMethod.JavaMethodZero(this.getSingletonClass(), Visibility.PUBLIC) { @Override public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name) { return ((JZlibRubyGzipWriter) self).realIo.callMethod(context, "path"); } }); } - return obj; + return this; } - + private static void checkLevel(Ruby runtime, int level) { - if (level < 0 || level > 9) { + if (level != JZlib.Z_DEFAULT_COMPRESSION && (level < JZlib.Z_NO_COMPRESSION || level > JZlib.Z_BEST_COMPRESSION)) { throw RubyZlib.newStreamError(runtime, "stream error: invalid level"); } } + @Override @JRubyMethod(name = "close") public IRubyObject close() { diff --git a/core/src/main/java/org/jruby/ext/zlib/RubyGzipFile.java b/core/src/main/java/org/jruby/ext/zlib/RubyGzipFile.java index 776d93d767b..542f8971d69 100644 --- a/core/src/main/java/org/jruby/ext/zlib/RubyGzipFile.java +++ b/core/src/main/java/org/jruby/ext/zlib/RubyGzipFile.java @@ -54,18 +54,6 @@ static IRubyObject wrapBlock(ThreadContext context, RubyGzipFile instance, Block return instance; } - static IRubyObject[] argsWithIo(IRubyObject io, IRubyObject[] args) { - List newArgs = new ArrayList(); - newArgs.add(io); - for (IRubyObject arg : args) { - if (arg == null) { - break; - } - newArgs.add(arg); - } - return newArgs.toArray(new IRubyObject[0]); - } - @JRubyMethod(meta = true, name = "wrap", compat = CompatVersion.RUBY1_8) public static IRubyObject wrap(ThreadContext context, IRubyObject recv, IRubyObject io, Block block) { Ruby runtime = recv.getRuntime(); diff --git a/core/src/main/java/org/jruby/ext/zlib/RubyZlib.java b/core/src/main/java/org/jruby/ext/zlib/RubyZlib.java index d5865ce76b2..8407e7fe8b1 100644 --- a/core/src/main/java/org/jruby/ext/zlib/RubyZlib.java +++ b/core/src/main/java/org/jruby/ext/zlib/RubyZlib.java @@ -341,4 +341,10 @@ static RaiseException newGzipFileError(Ruby runtime, String klass, String messag excn.setInstanceVariable("@input", runtime.getNil()); return new RaiseException(excn, true); } + + static int FIXNUMARG(IRubyObject obj, int ifnil) { + if (obj.isNil()) return ifnil; + + return RubyNumeric.fix2int(obj); + } }