Mapfile that is encoded as UTF-8 with BOM is not accepted #5194

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@tbonfort
Member

tbonfort commented Nov 7, 2015

Mapserver 7.0 does not accept mapfiles if they start with BOM. It seems that BOM has no effect in UTF-8 encoding http://www.unicode.org/faq/utf_bom.html#bom5 but some software still write it by default to text files, for example Notepad++
If mapfile starts with BOM Mapserver gives this error:
msLoadMap(): Unknown identifier. First token must be MAP, this doesn't look like a mapfile.
Issue can be tested with the attached mapfile that github did not accept it as "bom.map" so I renamed it into "bom.map.txt". It is easy to create by hand as well because the whole mapfile is just
MAP
END

Test query:
http://localhost/cgi-bin/mapserv.exe?map=bom.map

I think that making Mapserver to accept starting BOM could not break anything. Minimum action would be to mention the denial of BOM in the migration guide and in the mapfile documentation,

bom.map.txt

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Nov 7, 2015

Member

@jratike80 can try out the provided patch?
@sdlime can you review?

Member

tbonfort commented Nov 7, 2015

@jratike80 can try out the provided patch?
@sdlime can you review?

@tbonfort tbonfort changed the title from Mapfile that is encoded as UFT-8 with BOM is not accepted to Mapfile that is encoded as UTF-8 with BOM is not accepted Nov 7, 2015

@jratike80

This comment has been minimized.

Show comment
Hide comment
@jratike80

jratike80 Nov 7, 2015

Sorry, I can test only with precompiled binaries like the Windows development builds in gisinternals.com. The issue is well defined and I trust that you understand what you did.

Sorry, I can test only with precompiled binaries like the Windows development builds in gisinternals.com. The issue is well defined and I trust that you understand what you did.

@sdlime

This comment has been minimized.

Show comment
Hide comment
@sdlime

sdlime Nov 9, 2015

Member

@tbonfort, patch looks reasonable to me. I wonder if it's really necessary but since it's easy... Any reason we'd support other BOMs?

Member

sdlime commented Nov 9, 2015

@tbonfort, patch looks reasonable to me. I wonder if it's really necessary but since it's easy... Any reason we'd support other BOMs?

@jratike80

This comment has been minimized.

Show comment
Hide comment
@jratike80

jratike80 Nov 9, 2015

@sdlime, would you rather explain to Windows users why they can't create a proper mapfile with Notepad or why "UTF-8" selected from the menu of Notepad++ is not the correct one but they must select "UTF-8 without BOM" and so on? I think that this fix is friendly for the users and developers as well.

@sdlime, would you rather explain to Windows users why they can't create a proper mapfile with Notepad or why "UTF-8" selected from the menu of Notepad++ is not the correct one but they must select "UTF-8 without BOM" and so on? I think that this fix is friendly for the users and developers as well.

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Nov 9, 2015

Member

@sdlime no use in supporting other boms as we require mapfiles to be utf8 encoded

Member

tbonfort commented Nov 9, 2015

@sdlime no use in supporting other boms as we require mapfiles to be utf8 encoded

tbonfort added a commit that referenced this pull request Feb 24, 2016

@tbonfort

This comment has been minimized.

Show comment
Hide comment
@tbonfort

tbonfort Feb 24, 2016

Member

rebased to branch-7-0 in e50c067

Member

tbonfort commented Feb 24, 2016

rebased to branch-7-0 in e50c067

@tbonfort tbonfort closed this Feb 24, 2016

@tbonfort tbonfort added this to the 7.0.1 milestone Feb 24, 2016

@tbonfort tbonfort deleted the tbonfort:issues/5194-ignore-utf8-bom branch Feb 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment