Wikimedia Bugzilla is closed!

Try changing the flags in the proc_open call in LuaStandaloneEngine.php from 'r' and 'w' to 'rb' and 'wb', to specify binary mode.

It's not documented at http://www.php.net/proc_open, but it seems to have been in the PHP source since 2002.

(In reply to comment #1)
> Try changing the flags in the proc_open call in LuaStandaloneEngine.php from
> 'r' and 'w' to 'rb' and 'wb', to specify binary mode.
> 
> It's not documented at http://www.php.net/proc_open, but it seems to have
> been
> in the PHP source since 2002.

Flag change tried. I get the same error.

I just want to confirm the bug on my Windows XP machine.

--------------
Simple example
--------------

* Create Module:Bugs

* Add the following text

local z = {}

function z.new_line( frame )
    local s = "a\nb"
    return s
end

return z

* Invoke the Module with the following

{{#invoke:Bugs|new_line}}

* The following error will generate

<b>Notice</b>:  unserialize(): Error at offset 75 of 79 bytes...


-----------
Other notes
-----------

* rb / wb did not work

* adding the working fix cited above did work

* The behavior is not specifically related to PHP. I came across this bug in a Java app (XOWA).

The issue appears to be related to how the Lua binary returns "\n" on Windows machines.

* The recursiveEncode in MWServer.lua is clearly measuring a string of length 3:

a:1:{i:1;s:3:"a
b";}

* However, a "\r" somehow shows up in receiveMessage, and needs to be actively removed by the working fix

I'm going to guess that Lua is automatically converting all "\n" to "\r\n" when writing to pipe.read (stream.input) on Windows machines. If so, I think the only "true" fix is to somehow override Lua's default newline behavior on Windows machines. If that's not possible, then the working fix is probably the best alternative.

Hope this helps.

(In reply to comment #3)
> I'm going to guess that Lua is automatically converting all "\n" to "\r\n"
> when writing to pipe.read (stream.input) on Windows machines. If so, I think 
> the only "true" fix is to somehow override Lua's default newline behavior on
> Windows machines. If that's not possible, then the working fix is probably
> the best alternative.

Lua just uses fwrite() on stdout, i.e. the FILE* supplied by the C library called stdout. I don't think there is any way to change what mode it is in, and reopening it would be fairly complex. Note that "\r\n" might also be converted to \n by io.stdin:read() on the Lua side.

Choosing a serialization format that does not use line endings would be one way around it. Converting line endings as appropriate if a Windows Lua binary is detected would be another.

(In reply to comment #4)
> Lua just uses fwrite() on stdout, i.e. the FILE* supplied by the C library
> called stdout. I don't think there is any way to change what mode it is in,
> and
> reopening it would be fairly complex. 

Agreed. I got as far as "io.stdout:write" in "MWServer:sendMessage" and assumed it was Lua. It makes more sense that it is the underlying C library / OS.

> Note that "\r\n" might also be
> converted
> to \n by io.stdin:read() on the Lua side.

I don't think this should be an issue as "encodeLuaVar" in "LuaStandaloneEngine.php" escapes "\r" and "\n" during serialization

  case 'string':
    return '"' .
      strtr( $var, array( 
        '"' => '\\"',
        '\\' => '\\\\',
        "\n" => '\\n',
        "\r" => '\\r',
        "\000" => '\\000',
      ) ) .
      '"';

> Choosing a serialization format that does not use line endings would be one
> way
> around it.

If I understand correctly, are you saying that "\n", "\r" should be escaped on the Lua side (in "recursiveEncode")? If so, I think this is theoretically best, though it may involve a performance hit, which would be especially unnecessary for Linux machines.

> Converting line endings as appropriate if a Windows Lua binary is
> detected would be another.

I personally think this is best, at least for a short-term fix. "LuaStandaloneEngine.php" could detect the OS and apply the working fix if it is Windows. For what it's worth, I used a similar approach in my Java app.

Let's not screw around with OS detection, that just seems like a good way to have hard-to-track-down bugs.

Gerrit change #56425 just unconditionally escapes these characters for transport. Windows users, please test.

After checking out change #56425 and the manually copy of smw.property.lua into the LuaCommon\lualib folder PHPUnit would not crash on windows but return with some meaningful test reports.

I've tested Gerrit change #56425 
(last master revision + two changed code files)
it works for me
(Windows Vista -- Windows NT 6.0)


Tested :
- explicit \n in string
- long-brackets string
- mw.log()


Context :

- in edit-page's "Debug console"
  now OK
  before there was the output
  -----------------------------------------------------
  Lua error: Internal error: Unable to decode message.
  -----------------------------------------------------

- via {{#invoke:...}}
  now OK
  before the returned HTML contained
  -----------------------------------------------------
  <b>Notice</b>:  unserialize(): Error at offset 75 of 79 bytes in <b>[file path]\Scribunto\engines\LuaStandalone\LuaStandaloneEngine.php</b> on line <b>341</b><br>
  -----------------------------------------------------
  [file path] is placeholder for my actual file path

I've also tested Gerrit change #56425.

Specs: Windows XP SP 3, Apache 2.4, PHP 5.4, MediaWiki 1.21wmf11

Confirmed: simple example above now returns a string of "a\nb" (instead of failing).

I've also reviewed the commit diffs and they look fine by me.

Thanks.

Merged.

Change 95398 had a related patch set uploaded by MarkAHershberger:
(bug 46294) Fix for Windows text-mode file handles

https://gerrit.wikimedia.org/r/95398

Change 95398 merged by MarkAHershberger:
(bug 46294) Fix for Windows text-mode file handles

https://gerrit.wikimedia.org/r/95398

No open patches to review here (backport patch to REL_1_21 got merged), hence resetting status to RESOLVED FIXED. Backport_to_Stable flag might be set to "+" by hexmode.