Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JojoDiff 8.0.4 (Sept 2020) changed diff format #16

Open
Mgamerz opened this issue Oct 6, 2020 · 5 comments
Open

JojoDiff 8.0.4 (Sept 2020) changed diff format #16

Mgamerz opened this issue Oct 6, 2020 · 5 comments

Comments

@Mgamerz
Copy link

Mgamerz commented Oct 6, 2020

Don't know if this project is still maintained, but as of September 2020 there is an update to jdiff that produces slightly different patch files.
https://sourceforge.net/projects/jojodiff/files/

Existing diff-files will be handled correctly by the new version, 
but diff-files from the new version will not be handled correctly by previous versions !

I write my own implementation of JPatch for C# (however, my project is GPL, so I have no problems with the license) - I am still looking into seeing what's changed, but I think it should be noted that unless the patch code is updated, implementations of jpatch/janpatch may not work with the latest version of jdiff.

@Mgamerz
Copy link
Author

Mgamerz commented Oct 7, 2020

After reviewing the code, I didn't see any changes. I emailed Joris and he gave me this data on the updated implementation:

Hi,
The only change is that the ESC-MOD sequence is now the default when a new operation sequence is needed (at the start of a file or after an EQL, DEL or BKT operation). 
It's a small efficiency gain of 2 bytes, but also simplifies the code.
Thank's for using jdiff !
Best regards,
Joris Heirbaut

After reading this, the new code makes sense, at the beginning of the file read and the end of the operations the current opcode is set back to ESC. A simple change but it would indeed break parsing. This is just an FYI in case anyone wants to implement this in this version.

@janjongboom
Copy link
Owner

@Mgamerz Thanks for the update. I'm not actively maintaining this at the moment, but if someone reads this and opens a PR without looking at the GPL source I'd be happy to land it.

pbatard added a commit to pbatard/janpatch that referenced this issue Apr 6, 2022
This should solve janjongboom#16.
Note that this patch was produced without looking at the JojoDiff sourcecode.
@pbatard
Copy link

pbatard commented Apr 6, 2022

I have just created a PR to add support for the latest JojoDiff format. Note that since I discovered both janpatch and JojoDiff today, I have been very careful not to look at any part of the JojoDiff source code, so that there can't be any issue with the licensing.

For good measure, I have also updated blinky-k64f.patch to the latest JojoDiff format (but I have left blinky-k64f-reverse.patch in the old format). I have tested that both the new and old blinky-k64f.patch files produced the expected results.

Btw, I suggest adding a header with the license, author and a link to the project in janpatch.h, so that, when it is reused in another project, which is what I am planning to do, people can locate its source and licensing terms more easily.

Thanks again for creating janpatch!

@0x6d61726b
Copy link

I implemented the changes of this pull request and ran some tests. Unfortunately I could not find JojoDiff v0.8.4 but I found v0.8.5 which is a successor. Most test files passed patching and provided an 100% identical new-file, but a few of my test files with pseudo random data failed:

janpatch.exe pseudoRandom-0008192B.bin pseudoRandom-0008192B__to__pseudoRandom-0008460B.bin.jdiff pseudoRandom-0008460B.bin.janpatch-pbatard
  Unsupported operator b9
  Positions are, source=0 patch=2 new=0

janpatch.exe pseudoRandom-0010831B.bin pseudoRandom-0010831B__to__pseudoRandom-0011488B.bin.jdiff pseudoRandom-0011488B.bin.janpatch-pbatard
  Unsupported operator fd
  Positions are, source=0 patch=2 new=0

janpatch.exe pseudoRandom-0051210B.bin pseudoRandom-0051210B__to__pseudoRandom-0051555B.bin.jdiff pseudoRandom-0051555B.bin.janpatch-pbatard
  Unsupported operator b9
  Positions are, source=0 patch=2 new=0

janpatch.exe pseudoRandom-6731689B.bin pseudoRandom-6731689B__to__pseudoRandom-7084142B.bin.jdiff pseudoRandom-7084142B.bin.janpatch-pbatard
  Unsupported operator 3b
  Positions are, source=697149 patch=697196 new=697149

The content of the files can be found in janpatch-testfiles.zip.

I found that @mykhailo.cherkes has implemented a similar update at uniquwy/janpatch which produced the same output.

@akospasztor
Copy link

akospasztor commented Sep 23, 2024

Hi guys, so I took a look at the fix origintaing from @pbatard and ran some tests with real firmware binaries. I ended up having issues in some cases - i.e. with some patch binaries it worked perfectly, however in some cases the janpatch loop ended up in the default branch on line JANPATCH_ERROR("Unsupported operator %02x\n", c);. I strongly believe this is the same problem that @0x6d61726b faced with the pseudo-random tests.

Without looking at the source code to respect the GPL license, I adjusted the implementation so that it handles the default ESC-MOD operation described by @Mgamerz. I tested it with various "real-life" firmware binaries successfully (however I haven't run any random or pseudo-random tests yet).

My PR #22 contains only the minimum required changes without touching any other part of the code. I excluded further improvements focusing on readability and quality. If this PR gets approved, I will push those improvements if requested.

For anyone who is involved: can you please run tests on your ends as well to verify the functionality of the new processing logic to support the (breaking) changes introduced with v0.8.5?

Thanks a lot for all of your work and contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants