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

Fix bom #26

Merged
merged 3 commits into from
Oct 3, 2024
Merged

Fix bom #26

merged 3 commits into from
Oct 3, 2024

Conversation

pawelru
Copy link
Contributor

@pawelru pawelru commented Oct 2, 2024

I have a package that imports lzstring. I test my package on different OS including windows machines using RHub workflows. I got errors in (r-release-windows-x86_64) (R-release) and (r-oldrel-windows-x86_64) (R-oldrel).
The traceback is as follows:

Error: Error: Invalid byte order mark (BOM).
Backtrace:
    ▆
(...)
 5.   └─lzstring::compressToEncodedURIComponent(as.character(files_json)) at roxy.shinylive/R/parse_url.R:31:3
 6.     └─lzstring:::safe_compress(string, compressToEncodedURIComponent_)
 7.       └─lzstring (local) f(string_utf16)

It took me a while to find out what's going on. Even a simple lzstring::compressToEncodedURIComponent("Hello World!") failed.
I made a print the source of the lzstring:::convert_to_utf16le function and this is what I got:

function (string) 
{
    string <- enc2utf8(string)
    string_utf16 <- iconv(string, from = "UTF-8", to = "UTF-16LE", 
        toRaw = TRUE)[[1]]
    bom_le <- charToRaw("ÿþ")
    if (!identical(string_utf16[1:2], bom_le)) {
        string_utf16 <- c(bom_le, string_utf16)
    }
    string_utf16
}
<bytecode: 0x000001e0e794fc00>
<environment: namespace:lzstring>

In particular: bom_le <- charToRaw("ÿþ").

Then this happens:

> print(charToRaw("ÿþ"))
 [1] c3 bf c3 be

Then the if is false and c3 bf c3 be is appended:

> print(lzstring:::convert_to_utf16le("test"))
 [1] c3 bf c3 be 74 00 65 00 73 00 74 00

(as opposed to the expected ff fe 74 00 65 00 73 00 74 00 value)
Then this throws.

This PR is an attempt to fix this.

@parmsam
Copy link
Owner

parmsam commented Oct 3, 2024

Thanks for making the PR, @pawelru! I'll review this soon and merge it into the main branch. Will try to get a new version release onto CRAN before the end of the month.

@parmsam
Copy link
Owner

parmsam commented Oct 3, 2024

Plan to update pkgdown site manually. Will add you in the description file as a contributor. Merging into main now.

@parmsam parmsam merged commit df2304c into parmsam:main Oct 3, 2024
6 of 7 checks passed
@pawelru
Copy link
Contributor Author

pawelru commented Oct 3, 2024

Thanks for the prompt reply! I am definitely looking forward a CRAN release of that. I'm also planning to do CRAN release of my package and for this I might need that fix.

@pawelru pawelru deleted the fix_bom branch October 3, 2024 09:05
@parmsam-pfizer
Copy link
Collaborator

That’s awesome! I have a PR that would add Shinylive URL encode and decode functions into the Shinylive R package: posit-dev/r-shinylive#92 You may have seen it already. Hopefully, that’ll get reviewed and merged in before the end of the year.

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