New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 646107 link

Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression



Sign in to add a comment

secure shell not loading on canary

Project Member Reported by wfh@chromium.org, Sep 12 2016

Issue description

Version: 55.0.2858.0 canary (64-bit)
OS: Windows 10
Secure Shell version 0.8.34.

What steps will reproduce the problem?
(1) Try to open secure shell app (app id pnhechapfaindjhompbnflcldabbghjo)
(2) It doesn't load
(3)

What is the expected output?

Loads.

What do you see instead?

Doesn't load.

Please use labels and text to provide additional information.

I get this in Developer Tools output:

nassh_deps.concat.js:16224 Uncaught ReferenceError: FileError is not defined
    at Object.lib.fs.installFileErrorToString (nassh_deps.concat.js:16224)
    at nassh_deps.concat.js:16168
nassh_deps.concat.js:184 init: hterm
nassh_deps.concat.js:184 init: nassh
nassh_deps.concat.js:16224 Uncaught ReferenceError: FileError is not defined
    at Object.lib.fs.installFileErrorToString (nassh_deps.concat.js:16224)
    at nassh_deps.concat.js:16168
nassh_deps.concat.js:5248 Uncaught TypeError: Failed to execute 'postMessage' on 'Window': The 3rd argument is neither an array, nor does it have indexed properties.
    at hterm.Frame.onLoad_ (nassh_deps.concat.js:5248)

This was working last week so a it's recent regression.
 

Comment 1 by wfh@chromium.org, Sep 12 2016

Cc: rginda@chromium.org

Comment 2 by vapier@chromium.org, Sep 13 2016

55.0.2853.0 is working, but this is under Linux

Comment 3 by wfh@chromium.org, Sep 13 2016

55.0.2857.0 is also not working (other machine).

Comment 4 by wfh@chromium.org, Sep 13 2016

Cc: rdevlin....@chromium.org
Labels: -Pri-3 OS-All Pri-1
You are probably looking for a change made after 417040 (known good), but no later than 417070 (first known bad).
CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/9773f67..08557a2?pretty=fuller&n=1000

Comment 5 by wfh@chromium.org, Sep 13 2016

Cc: -rginda@chromium.org -rdevlin....@chromium.org haraken@chromium.org sigbjo...@opera.com jsb...@chromium.org dmazz...@chromium.org aboxhall@chromium.org
Owner: foolip@chromium.org
Status: Assigned (was: Untriaged)
further bisect to 

https://chromium.googlesource.com/chromium/src/+log/41481c7..77a58a9?pretty=fuller&n=1000

 https://codereview.chromium.org/2295863002

I presume hterm is using the deprecated API but I don't know how to contact the hterm developers. foolip@ can you investigate, as this will likely impact quite a few users.

Comment 6 by jsb...@chromium.org, Sep 13 2016

Cc: rginda@chromium.org
Isn't rginda the hterm author?

The hterm source is here, I'm not totally positive if this is the canonical source, but it's a good starting point:

https://chromium.googlesource.com/chromiumos/platform/assets/+/5317b308ad4af713d7eeabf864d6e85865f032ed/chromeapps/hterm/

Comment 8 by jsb...@chromium.org, Sep 13 2016

https://chromium.googlesource.com/apps/libapps/+/HEAD/hterm

And yeah, there are a couple of postMessage(data, transferrables, origin)-ordered calls in there:

https://chromium.googlesource.com/apps/libapps/+/HEAD/hterm/js/hterm_frame.js

Comment 9 by vapier@chromium.org, Sep 13 2016

the authors are on the bug

the hterm source is that libapps repo
Cc: foolip@chromium.org
Owner: rginda@chromium.org
The fix for this is to change the order of [this.messageChannel_.port2] and this.url in the postMessage arguments.

rginda@, can you see about making that change? This change has been backported to M54, so if it's not possible to deploy an update in time for M54 stable, we'll have to delay the removal. If so, by how much? (If we do that it'd be nice to let it sit for a few more weeks to shake out more regressions and then revert, so that next time has a better chance of working out.)

Comment 11 by wfh@chromium.org, Sep 14 2016

I can have a go hacking a fix to this into libapps. I could only find one place that this had to be changed.
Cc: vapier@chromium.org
Owner: vapier@chromium.org
if someone wants to send us a CL, should be easy to get it in

Comment 13 by wfh@chromium.org, Sep 14 2016

I can't get the hterm tests to pass on any Chromium build, I tried before or after the change in #5 and also tried a version of Chromium corresponding to stable. They pass on Google Chrome Stable M53.

Chromium 55.0.2854.0 (417054) (failed)
Chromium 55.0.2855.0 (417390) (failed)
Chromium 53.0.2785.0 (403380) (failed)
Chrome 53.0.2785.113 (Official Build) (pass)

Not sure if hterm tests require something that is only in Chrome but without an assurance I can actually verify the failure and then verify the fix, I'm not really happy landing a change to herm blind.
the tests are passing for me.  if you upload the CL i can check on my side.

Comment 15 by wfh@chromium.org, Sep 14 2016

https://chromium-review.googlesource.com/#/c/385194/

Which revision did the tests pass in? if they are passing in HEAD, how will you be able to verify that this change fixes the issue?
i'm using HEAD (7354202ec171287e200ec075c40e34784005fa97).  adding your commit passes with:
  53.0.2785.101
  54.0.2840.16
  55.0.2853.0

i don't know why it's failing on your side, but if it's passing for me, not sure it's interesting to chase it down ;).

i'll double check SecureShell now.
Project Member

Comment 17 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/b976c0b956121d3e10934dea16d61417b6b17f1d

commit b976c0b956121d3e10934dea16d61417b6b17f1d
Author: Will Harris <wfh@chromium.org>
Date: Wed Sep 14 20:44:59 2016

hterm: fix order of parameters to postMessage

The window.postMessage argument order we were using is deprecated and
has been removed in newer Chrome versions.  Switch the arguments to
match the current API.

BUG= chromium:646107 

Change-Id: I76564daa03efcfcb18a30593be26be13a6fa11f9
Reviewed-on: https://chromium-review.googlesource.com/385194
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/b976c0b956121d3e10934dea16d61417b6b17f1d/hterm/js/hterm_frame.js

also verified tests pass with:
  54.0.2840.16
  55.0.2859.0

and that SecureShell works again with 55.0.2853.0 & 55.0.2859.0
Awesome! Will there be an update of SecureShell for all users in time for M54 stable?
i've published 0.8.34.3 to the dev extension.  if i don't see any reports, i'll promote it to the stable one.  but i'm going to prob wait until monday for that (since i'll be out this weekend).

Comment 21 by wfh@chromium.org, Sep 16 2016

 Issue 647730  has been merged into this issue.

Comment 22 Deleted

Comment 23 by wfh@chromium.org, Sep 16 2016

reiber@ need more version information to diagnose your issue. can you provide the version of Chrome, the version of your OS and the version of Secure Shell?

Comment 24 by rei...@gmail.com, Sep 16 2016

nassh (non-dev) was working fine for me until today.  0.8.34.2 comes up with the connection settings dialog in a wrong font, no data loads into it - it's hung.

It worked great yesterday.

Comment 25 by rei...@gmail.com, Sep 16 2016

This is on an HP Chromebook:

Version 55.0.2858.0 dev (64-bit)
Platform 8798.0.0 (Official Build) dev-channel falco
Firmware Google_Falco.4389.92.0

Comment 26 by wfh@chromium.org, Sep 16 2016

Probably your chromebook updated to latest dev - the commit that changed the behavior was in 55.0.2855.0, and 55.0.2858.0 was released to dev channel yesterday [1] so I expect you just rebooted.

[1] https://googlechromereleases.blogspot.com/2016/09/dev-channel-update-for-chrome-os_15.html
ok, stupid question. how can i update secure shell to dev version? because if by enabling developer mode in extensions and clicking update extensions now, it doesn't seem to work.

Comment 28 by rei...@gmail.com, Sep 16 2016

Yes - I shut it down last night and booted up this morning to encounter this problem.  Changing to the 'beta' channel - if that doesn't work I'll let you know.  

Thank you!

Comment 29 by rei...@gmail.com, Sep 16 2016

Switching to the beta channel helped.  Thanks!

i suspect you're misremembering or testing things.  the SecureShell stable update did not break SecureShell on newer CrOS versions -- it was already broken.
Status: Fixed (was: Assigned)
i've pushed 0.8.34.4 to the stable extension, so should be all set now

Comment 32 by sush...@gmail.com, Sep 19 2016

I can confirm it works in Dev after the update. Thanks! 

Comment 33 by ka...@chromium.org, Sep 20 2016

 Issue 647540  has been merged into this issue.

Sign in to add a comment