Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11486 closed defect (fixed)

Fix FromWChar implementation

Reported by: kcwu Owned by:
Priority: normal Milestone:
Component: base Version:
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

Original implementation has some issues:

  • it may overflow dst buffer by 1 null character
  • when convert multibyte characters, it may not fill nul charaters because

src+lenChunk should be src+wxWcslen(src)

This patch also fix comment of FromWChar in strconv.h.

Attachments (1)

wx-bug-fromwchar.diff download (8.0 KB) - added by kcwu 4 years ago.

Download all attachments as: .zip

Change History (4)

Changed 4 years ago by kcwu

comment:1 Changed 4 years ago by VZ

(In [62792]) Minor corrections to ToWChar() comment.

Don't refer to the non-existent outputBuf parameter and don't imply that the
value of dstLen matters to decide whether we really convert or not -- only
whether dst pointer itself is NULL or not does.

See #11486.

comment:2 Changed 4 years ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [62793]) Correct two bugs in wxMBConv::FromWChar() with non NUL-terminated strings.

The variable "lenChunk" was incorrectly used as the length of the wide string
chunk which could result in wrong output.

Worse, the output buffer could be overflown for the final chunk because it
didn't have to have enough space for the trailing NUL(s) in it.

Fix both bugs and added unit tests for them.

Based on patch by Kuang-che Wu.

Closes #11486.

comment:3 Changed 4 years ago by vadz

Thanks a lot for your patches! It took me some time to realize that the bug could only be seen under Windows (under Unix, iconv converter directly implements FromWChar() instead of passing by WC2MB() and in fact it would be nice if Win32 converter could do this too, see #11518) which explains the delay as I didn't see the problem when initially testing under Linux.

Anyhow, I've applied a somewhat modified version of the patch which is a bit closer to ToWChar() (although these functions still have somehow become annoyingly different) which seems to work, i.e. your new unit tests (thanks!) pass. Please let me know if you find any problems!

Note: See TracTickets for help on using tickets.