Skip to content

Handle ZWNBSP characters #166

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

JackStuart
Copy link
Contributor

@JackStuart JackStuart commented Jan 18, 2025

Fixes handling ZWNBSP characters at the start of a file - Seems to be very common in XML files

Renamed File2 to File1 due to there being no File1.xml - So there was no point skipping to File2

Fixed typo of declaratioon

Resolves the issue here - mandiant/flare-vm#640

Fixes handling ZWNBSP characters at the start of a file - Seems to be very common in XML files
@flwyd
Copy link
Collaborator

flwyd commented Apr 26, 2025

Thanks for highlighting this issue!

I think a little more work needed to properly handle byte order marks. Skipping the first line if the file starts with a BOM isn't necessarily correct: it works for file types covered by the head/hashBang list (like XML and shell scripts). But for types without this first-line-is-important feature, the copyright should go on the first line, right after the BOM.

Please add a unit test which covers at least

  • XML: {BOM}<?xml\nLicense\n<!-- Normal comment -->
  • Shell script or other language with a shebang: {BOM}#!/bin/sh\nLicense\n# Normal comment
  • Octothorpe without a shebang: {BOM}License\n# Normal comment
  • Non-octothorpe language, e.g. C++: {BOM}License\n// Normal comment

Since Go strings are UTF-8, these tests will need to use byte arrays directly, with big-endian and little-endian versions of the BOM sequence.

@flwyd
Copy link
Collaborator

flwyd commented Jul 7, 2025

Friendly ping @JackStuart regarding the comments above.

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.

2 participants