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

Fixed "invalid" hash sizes (map protection?) in Broodwar maps #32

Merged
merged 2 commits into from
Feb 6, 2020

Conversation

Bytekeeper
Copy link
Contributor

Not sure about Warcraft, but some Starcraft maps use the upper 4 bit to make loading the mpq impossible. (The upper 4 bit are dropped because of * 16 in the game/hashtable - but JMPQ3 uses the original hashSize value to create the table)

@coveralls
Copy link

coveralls commented Feb 6, 2020

Coverage Status

Coverage increased (+0.7%) to 85.586% when pulling e624189 on Bytekeeper:master into 70e8e86 on inwc3:master.

@Frotty
Copy link
Member

Frotty commented Feb 6, 2020

Cool, thanks for the PR 👍
Could you upload a starcraft brood war test map/mpq for this case? (ideally small size with all data removed).
And I think the current change has no effect - did you test this? IntelliJ reports it as superfluous:

I think it should be & 0xFFFFFFFFL as the other above, for which it would need to be a long?

@DrSuperGood do you know something about this?

@Bytekeeper
Copy link
Contributor Author

Bytekeeper commented Feb 6, 2020

@Frotty I'm puzzled, why does it show 8 Fs for you? Should be 7 (it even has the prefix 0x0... in my commit) :) Will try to find a small example.

@Frotty
Copy link
Member

Frotty commented Feb 6, 2020

Oops, I must have mistakenly added one during copy+paste as I didn't check out the branch 😬
Alright, so it seems good. Any thoughts about the test map?

@Bytekeeper
Copy link
Contributor Author

Bytekeeper commented Feb 6, 2020

I have a test map, but it makes the "testDuplicatePaths" test fail. Ok if I skip that map (it's a SC map after all) - or do you want a failing test ;)

java.nio.BufferOverflowException
	at systems.crigges.jmpq3.MpqFile.writeFileAndBlock(MpqFile.java:346)
	at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:811)
	at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:705)
	at systems.crigges.jmpq3.JMpqEditor.close(JMpqEditor.java:701)
	at systems.crigges.jmpq3test.MpqTests.testDuplicatePaths(MpqTests.java:252)

@Frotty
Copy link
Member

Frotty commented Feb 6, 2020

I don't know which test you are referring to with "testDuplicatePaths".
Of course there should not be a failing test 😄 . Why exactly is it failing?

@Bytekeeper
Copy link
Contributor Author

Bytekeeper commented Feb 6, 2020

Uhm, check the trace above. The test is systems.crigges.jmpq3test.MpqTests#testDuplicatePaths. I do not know why it fails. I would guess it is just one further "protection" to make editing the map fail. (Besides having an invalid hash size no (listfile) etc etc.)

@Frotty
Copy link
Member

Frotty commented Feb 6, 2020

Okay I wasn't on latest master on this machine. 😅
So this PR makes it readable but not modifiable?
If only that test fails, which seems weird, I suppose you could omit the starcraft map there.

e: and yea, this looks like the usual bugs with non trivial protection.

@Bytekeeper
Copy link
Contributor Author

Not sure, maybe it is modifiable. I think that would need further analysis. My use case really only needs reading for now. I'll add the map as an exception.

@Frotty
Copy link
Member

Frotty commented Feb 6, 2020

Nice 👍
What is your use case? Are you using jmpq in some project.
Gonna merge.

@Frotty Frotty merged commit d782af0 into inwc3:master Feb 6, 2020
@Bytekeeper
Copy link
Contributor Author

@Frotty Yeah - I'm part of a project to enable players to play vs custom AIs for Starcraft:
for this project https://makingcomputerdothings.com/schnail-starcraft-human-n-ai-league-development-roadmap-extra-patreon-rewards/
I'm using it to load maps and render a preview inside the client part. Many maps in SC are modified to prevent alteration and make it crash in an editor. Things like the invalid hash size which work in-game but cannot be loaded by an editor.

@Frotty
Copy link
Member

Frotty commented Feb 7, 2020

Sounds cool 😄
There are also many protections like this for wc3 maps, but I'm not too interested in supporting them.
Please don't forget to star the repo if you deem it useful in your projects.
Cheers 🍻

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

Successfully merging this pull request may close these issues.

3 participants