1024jp / GzipSwift

Swift package that enables gzip/gunzip Data using zlib
MIT License
544 stars 134 forks source link

Not supported in swift 5 #34

Closed philmartin83 closed 5 years ago

philmartin83 commented 5 years ago

Testing this in Swift 5 and this seems to crash on deflate(&stream, Z_FINISH) line 212

Is this library going to be compatible with Swift 5 at all

Thanks

1024jp commented 5 years ago

Oops, it's just because I don't have time yet. I'll try it later.

philmartin83 commented 5 years ago

I've found replacing this

    var data = Data(capacity: DataSize.chunk)
    while stream.avail_out == 0 {
        if Int(stream.total_out) >= data.count {
            data.count += DataSize.chunk
        }
        data.withUnsafeMutableBytes { (bytes: UnsafeMutablePointer<Bytef>) in
            stream.next_out = bytes.advanced(by: Int(stream.total_out))
        }
        stream.avail_out = uInt(data.count) - uInt(stream.total_out)

        deflate(&stream, Z_FINISH)
    }

    deflateEnd(&stream)
    data.count = Int(stream.total_out)

with this

   var data = Data(capacity: DataSize.chunk)
    while stream.avail_out == 0 {
        if Int(stream.total_out) >= data.count {
            data.count += DataSize.chunk
        }

        data.withUnsafeMutableBytes { (buffer) in
            let base = buffer.baseAddress
            stream.next_out = base?.advanced(by: Int(stream.total_out)) as? UnsafeMutablePointer<Bytef>
        }

       //            UnsafeMutableBufferPointer
       //
       //            data.withUnsafeMutableBytes { (bytes: UnsafeMutablePointer<Bytef>) in
       //                stream.next_out = bytes.advanced(by: Int(stream.total_out))
       //            }
        stream.avail_out = uInt(data.count) - uInt(stream.total_out)

        deflate(&stream, Z_FINISH)
    }

    deflateEnd(&stream)
    data.count = Int(stream.total_out)

Fixes the crash I do however get a warning.

there also other warnings 'withUnsafeBytes' is deprecated: use withUnsafeBytes<R>(_: (UnsafeRawBufferPointer) throws -> R) rethrows -> R instead

Hopefully this points you in the right direction. Feel free to test what I've tried.

palmcrust commented 5 years ago

Any progress with that?

To make the things worse both gzipped gunzipped crash with XCode 10.2 even if I leave the code untouched and have it as 4.2 compilant, while with XCode 10.1 the same code works without any problems. Is it a bug in either compiler, or Swift library? Probably, this is how it is meant to be in the future releases.

I converted to Swift 5 and did a replacement very similar to what philmartin83 suggested

   stream.next_out =   data.withUnsafeBytes({
        (bytes: UnsafeRawBufferPointer)-> UnsafeMutablePointer<Bytef> in
            UnsafeMutablePointer(mutating: 
                bytes.bindMemory(to: Bytef.self).baseAddress!.advanced(by: Int(stream.total_out)))
     })

for gzipped and gunzipped

No compiler warnings and no crashes at runtime, however the restored data are different from the original.

I can't use libcompression, as it supports ios 10+, while my app runs with ios 9+ . (I keep an old iphone and wish it to run there as well)

Unfortunately I am quite new to Swift and never dealt with zlib. Also, zlib targets C, where working with pointers is much simpler than in Swift.

Hopefully, someone will find time to fixed that. I really need it to work.

ArchieR7 commented 5 years ago

same here!

deflate(&stream, Z_FINISH) //Thread 1: EXC_BAD_ACCESS (code=1, address=0x196ddfa00)
1024jp commented 5 years ago

Sorry for the pending. However, I couldn't found the clue so far and am recently too busy to work with this. I'll reseach between whiles, but a PR is welcome.

anthann commented 5 years ago

Any progress with that?

To make the things worse both gzipped gunzipped crash with XCode 10.2 even if I leave the code untouched and have it as 4.2 compilant, while with XCode 10.1 the same code works without any problems. Is it a bug in either compiler, or Swift library? Probably, this is how it is meant to be in the future releases.

I converted to Swift 5 and did a replacement very similar to what philmartin83 suggested

   stream.next_out =   data.withUnsafeBytes({
        (bytes: UnsafeRawBufferPointer)-> UnsafeMutablePointer<Bytef> in
            UnsafeMutablePointer(mutating: 
                bytes.bindMemory(to: Bytef.self).baseAddress!.advanced(by: Int(stream.total_out)))
     })

for gzipped and gunzipped

No compiler warnings and no crashes at runtime, however the restored data are different from the original.

I can't use libcompression, as it supports ios 10+, while my app runs with ios 9+ . (I keep an old iphone and wish it to run there as well)

Unfortunately I am quite new to Swift and never dealt with zlib. Also, zlib targets C, where working with pointers is much simpler than in Swift.

Hopefully, someone will find time to fixed that. I really need it to work.

same here! Something goes wrong with XCode 12.2 and/or libraries shipped with it.

philmartin83 commented 5 years ago

Any progress with that? To make the things worse both gzipped gunzipped crash with XCode 10.2 even if I leave the code untouched and have it as 4.2 compilant, while with XCode 10.1 the same code works without any problems. Is it a bug in either compiler, or Swift library? Probably, this is how it is meant to be in the future releases. I converted to Swift 5 and did a replacement very similar to what philmartin83 suggested

   stream.next_out =   data.withUnsafeBytes({
        (bytes: UnsafeRawBufferPointer)-> UnsafeMutablePointer<Bytef> in
            UnsafeMutablePointer(mutating: 
                bytes.bindMemory(to: Bytef.self).baseAddress!.advanced(by: Int(stream.total_out)))
     })

for gzipped and gunzipped No compiler warnings and no crashes at runtime, however the restored data are different from the original. I can't use libcompression, as it supports ios 10+, while my app runs with ios 9+ . (I keep an old iphone and wish it to run there as well) Unfortunately I am quite new to Swift and never dealt with zlib. Also, zlib targets C, where working with pointers is much simpler than in Swift. Hopefully, someone will find time to fixed that. I really need it to work.

same here! Something goes wrong with XCode 12.2 and/or libraries shipped with it.

When I've got this setup for Swift 4.2 in XCode 10.2 it works fine its when I set it Swift 5 it crashes.

georgbachmann commented 5 years ago

Same issue here... would be thankful for a fix as I have no idea about gzip and can't contribute... sorry...

philmartin83 commented 5 years ago

this doesn't help anyone but I've tired this on a device running iOS 12.2 and iOS 12.1.4, on the 12.2 its works ok for me on the device running 12.1.4 its crashes has anyone else found this?

georgbachmann commented 5 years ago

this doesn't help anyone but I've tired this on a device running iOS 12.2 and iOS 12.1.4, on the 12.2 its works ok for me on the device running 12.1.4 its crashes has anyone else found this?

Yep.. exact same behaviour for me. Also the code you provided on top seems to make invalid gzipped data? Again... I don't know much about zipping, but my server at least is rejecting my gzipped data using your workaround :(

philmartin83 commented 5 years ago

I'm not an expert in this, its just something I read.

georgbachmann commented 5 years ago

Sure... was just an observation :) thank for sharing anyway!

philmartin83 commented 5 years ago

ok I think I've worked it out.....(I think).... I think its to do with the exclusive memory access. try this for the gzipped

public func gzipped(level: CompressionLevel = .defaultCompression) throws -> Data {

    guard !self.isEmpty else {
        return Data()
    }

    let contiguousData = self.withUnsafeBytes { Data(bytes: $0, count: self.count) }
    var stream = contiguousData.createZStream()
    var status: Int32

    status = deflateInit2_(&stream, level.rawValue, Z_DEFLATED, MAX_WBITS + 16, MAX_MEM_LEVEL, Z_DEFAULT_STRATEGY, ZLIB_VERSION, Int32(DataSize.stream))

    guard status == Z_OK else {
        // deflateInit2 returns:
        // Z_VERSION_ERROR  The zlib library version is incompatible with the version assumed by the caller.
        // Z_MEM_ERROR      There was not enough memory.
        // Z_STREAM_ERROR   A parameter is invalid.

        throw GzipError(code: status, msg: stream.msg)
    }

    var data = Data(capacity: DataSize.chunk)
    while stream.avail_out == 0 {
        var copyOfStream = stream
        if Int(copyOfStream.total_out) >= data.count {
            data.count += DataSize.chunk
        }
        //            if #available(iOS 12.2, *) {
        data.withUnsafeMutableBytes { (bytes: UnsafeMutablePointer<Bytef>) in
            copyOfStream.next_out = bytes.advanced(by: Int(stream.total_out))
        }

        copyOfStream.avail_out = uInt(data.count) - uInt(stream.total_out)
        deflate(&copyOfStream, Z_FINISH)
        stream = copyOfStream
    }

    deflateEnd(&stream)
    data.count = Int(stream.total_out)

    return data
}
georgbachmann commented 5 years ago

This still crashes for me under iOS 10

philmartin83 commented 5 years ago

@georgbachmann is it at the same point? I didn’t test on iOS 10 because I don’t have a device running that. I have a iOS 9 device I’ll try agains that later.

philmartin83 commented 5 years ago

@georgbachmann I cannot replicate your issue with iOS 10, I've tested on a iOS 10.3.3, iOS 9.3.5, iOS 11.3, iOS 12.1.4 and iOS 12.2 it all works fine since the change above.

I am however only using the .swift file I'm not installing via cocoa pod or using the framework. So as to why this doesn't work for you has exhausted my knowledge now sorry.

subdigital commented 5 years ago

@philmartin83 did you run the tests with your suggested change? It seems to produce empty data for me, and so testGZip() fails.

philmartin83 commented 5 years ago

I tested using the above yes, the data reported to our server seems to be correct. Again I’m not an expert on this matter.

subdigital commented 5 years ago

Do you have a fork I can test out? On Xcode 10.1 the tests pass. On Xcode 10.2 they crash in the same way described in the initial report.

Applying your changes and running the tests in the project and they fail because it products empty data. (Maybe a typo in the posted code, I'm not sure)

philmartin83 commented 5 years ago

No I don’t, I just updated it in the file in my project and then copy and pasted the above into this post.

subdigital commented 5 years ago

@philmartin83 If you could fork the project and make the changes there it would be really helpful to the rest of us (and you'd be able to verify your fix via the tests).

philmartin83 commented 5 years ago

my fix doesn't work. I've forked it and it doesn't work. I've exhausted my knowledge

samanthamjohn commented 5 years ago

@philmartin83 thanks for taking a stab. I appreciate your attempts, even if they don't work! I'm just going back to Xcode 10.1

philmartin83 commented 5 years ago

@samjohn no problem, the unit test fails on the gzip test as the uncompressed data converted to string is nil which is odd because the data before being compressed is 3 bytes and the uncompressed data is 3 bytes string is nil. You're more then welcome to take a look at my fork and see I'll push my changes now to my forked version.

https://github.com/philmartin83/GzipSwift

pouty18 commented 5 years ago

@1024jp any chance you can give us an ETA for when you plan on working on the migration to Swift 5?

ldrr commented 5 years ago

Thanks @philmartin83, you made my day

zhs2472 commented 5 years ago

@philmartin83 Great job! Thanks!

mjholgate commented 5 years ago

Hi guys, I have an alternate fix which fixes other areas of code that are broken in a similar way (and fixes unit tests).

I'm testing it at the moment but will push a branch and PR shortly.

philmartin83 commented 5 years ago

@mjholgate amazing work thanks.

mjholgate commented 5 years ago

Branch is here:

https://github.com/mjholgate/GzipSwift/tree/fix-issue-34-xcode-10.2

I still need to check this over again, and when I'm happy I'll submit a PR. Any comments/testing much appreciated.

mjholgate commented 5 years ago

Podfile runes to use the fork:

pod "GzipSwift" , :git => 'https://github.com/mjholgate/GzipSwift', :branch => 'fix-issue-34-xcode-10.2'

RobertGummesson commented 5 years ago

@mjholgate - The gzipped() function is still crashing for me in my Swift 5 project. Started debugging it but it wouldn't stop on my exception breakpoint. I'm at work so don't have time debugging any further. Perhaps not a super helpful comment but thought I'd mention it.

mjholgate commented 5 years ago

@RobertGummesson :-(. Just. checking - you definitely have the right branch?

adamwaite commented 5 years ago

If people haven't time to investigate and require a quick-fix, this library does the same job (amongst other compression functions), has a very similar API, and has been successfully ported to Swift 5 without any warnings.

https://github.com/mw99/DataCompression

1024jp commented 5 years ago

Hi, sorry for my late. But I improved my code based-on @philmartin83's fork (Thank you!). I've committed them to develop branch. Could you somebody test it? If it works, I'll merge it to the main branch.

philmartin83 commented 5 years ago

@1024jp I’m glad it was some sort of help.

dszakon commented 5 years ago

If just tested in my project, and it works

1024jp commented 5 years ago

Thanks for checking. I've released v5.0.0 that supports Swift 5 🚀.

sunnyleeyun commented 5 years ago

@1024jp I'm still encountering the error. Same crash on

deflate(&stream,` Z_FINISH) line 212

diegotl commented 5 years ago

Thanks! v5.0.0 fixed it.

@sunnyleeyun make sure to specify the new version in Podfile as CocoaPods still fetches v4.1.0: pod 'GzipSwift', :git => 'https://github.com/1024jp/GzipSwift.git', :tag => '5.0.0'

dszakon commented 5 years ago

I think you have to change the pod version to be 5.0.0, because now we have version 4.1.0 with tag 5.0.0

pbush25 commented 5 years ago

Thank you for fixing this! I thought I was going insane when I started seeing crashes pointing to deflate.

hopy11 commented 5 years ago

I use GzipSwift with tag 5.0.0 Still crash in Swift 5.0 (iOS 12.2) :(

KomarovEV commented 5 years ago

Still crashing. 11.0 beta 2 iOS 13