BrettMayson / arma-rs

GNU General Public License v2.0
42 stars 8 forks source link

Fix write_cstr overflow check #26

Closed Eathox closed 2 years ago

Eathox commented 2 years ago

Fixes issue in write_cstr where it treats buf_size as the amount of bits even thought this represents the amount of bytes, mentioned in: https://github.com/BrettMayson/arma-rs/issues/24#issuecomment-1109522273. After merging this effectively allows for a 8 times bigger return.

codecov[bot] commented 2 years ago

Codecov Report

Merging #26 (7d0122f) into main (3425f94) will increase coverage by 1.09%. The diff coverage is 98.30%.

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
+ Coverage   81.92%   83.02%   +1.09%     
==========================================
  Files          17       17              
  Lines         758      807      +49     
==========================================
+ Hits          621      670      +49     
  Misses        137      137              
Impacted Files Coverage Δ
arma-rs-proc/src/lib.rs 0.00% <ø> (ø)
arma-rs/src/lib.rs 53.06% <96.87%> (+18.77%) :arrow_up:
arma-rs/src/context.rs 100.00% <100.00%> (ø)
arma-rs/src/testing.rs 84.61% <100.00%> (ø)
arma-rs/tests/main.rs 97.27% <100.00%> (+0.33%) :arrow_up:
arma-rs/src/command.rs 87.25% <0.00%> (+0.12%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3425f94...7d0122f. Read the comment docs.

jonpas commented 2 years ago

Return buffer size in Arma 3 with CallExtensionArgs is currently 20480.

Right now arma-rs only allows return of 2559 bytes. Is this potentially the same issue? By BIKI it should be 20480 (-8) bytes.

Eathox commented 2 years ago

Where did you find the number 20480? I looked around but could only find that on the callExtension wiki it states that 10240 bytes is the buffer size:

Currently there is no limit how much data you can send to the extension. However there is a limit on how much data you can return from extension in one call. The limit is known to the extension and is passed in int outputSize. The limit may or may not change in the future and is currently 10240 bytes. It is up to extension designer to handle multipart results if returned data exceeds output limit.

Also I think there might a bit of a misunderstanding, the size limit should not be subtracted by 8. Since the size is given us to as the amount of bytes subtracting by 1 should be all that's needed to accommodate for a terminating zero at the end.

BrettMayson commented 2 years ago

Where did you find the number 20480?

@dedmen, but yes the wiki does say 10240 still

It only matters for testing though, the values is passed in from Arma when actually being used

jonpas commented 2 years ago

Also I think there might a bit of a misunderstanding, the size limit should not be subtracted by 8. Since the size is given us to as the amount of bytes subtracting by 1 should be all that's needed to accommodate for a terminating zero at the end.

You are probably correct there, I only tried not subtracting which lead to a crash, or subtracting 8, didn't try just subtracting 1. I am using it to cut long data into pieces for sending back, didn't test within arma-rs source itself.