Created attachment 277315 [details] crash test case ftp links crash @ WebCore::FTPDirectoryDocumentParser::appendEntry
<rdar://problem/24292650>
Created attachment 277321 [details] Patch
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review > Source/WebCore/ChangeLog:10 > + one. Can you clarify why?
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review > Source/WebCore/html/FTPDirectoryDocument.cpp:115 > + Ref<Element> nameElement = createTDForFilename(filename); Isn't it OK to assign to a Ref<> after it's been moved out?
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review >> Source/WebCore/ChangeLog:10 >> + one. > > Can you clarify why? Please explain in the changelog that Ref<>'s operator=() does not allow assignment after a WTFMove(). >> Source/WebCore/html/FTPDirectoryDocument.cpp:115 >> + Ref<Element> nameElement = createTDForFilename(filename); > > Isn't it OK to assign to a Ref<> after it's been moved out? Ok, I have just seem the implementation of Ref& operator=(T& object) and it clearly does not allow assignment after a move :/
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review > Source/WebCore/ChangeLog:12 > + No changes of behavior. Why no test?
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review Thanks Chris for reviewing this patch. >> Source/WebCore/ChangeLog:12 >> + No changes of behavior. > > Why no test? I don't know if it is possible to make a ftp test. Do you know how? >>> Source/WebCore/html/FTPDirectoryDocument.cpp:115 >>> + Ref<Element> nameElement = createTDForFilename(filename); >> >> Isn't it OK to assign to a Ref<> after it's been moved out? > > Ok, I have just seem the implementation of Ref& operator=(T& object) and it clearly does not allow assignment after a move :/ There is an ASSERT in operator= discouraging this usage. Here is the crash log. Process: com.apple.WebKit.WebContent.Development [8919] Path: /Users/USER/Documents/*/WebKit.framework/XPCServices/com.apple.WebKit.WebContent.Development.xpc/com.apple.WebKit.WebContent.Development Identifier: com.apple.WebKit.WebContent.Development Version: 602+ (602.1.30+) Code Type: X86-64 (Native) Parent Process: launchd_sim.development [6184] Responsible: com.apple.WebKit.WebContent.Development [8919] User ID: 501 Date/Time: 2016-04-25 19:16:49.008 -0700 OS Version: Mac OS X 10.11.5 (15F27) Report Version: 11 Anonymous UUID: 959E954D-4D93-D4D4-8B62-15433989F34D Sleep/Wake UUID: D5FEE3D1-A7DD-4FD0-A49D-825F10DE5647 Time Awake Since Boot: 46000 seconds Time Since Wake: 27000 seconds System Integrity Protection: enabled Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x00000000bbadbeef Exception Note: EXC_CORPSE_NOTIFY VM Regions Near 0xbbadbeef: --> __TEXT 0000000104412000-0000000104414000 [ 8K] r-x/rwx SM=COW /Users/USER/Documents/*/WebKit.framework/XPCServices/com.apple.WebKit.WebContent.Development.xpc/com.apple.WebKit.WebContent.Development Application Specific Information: CoreSimulator 245 - Device: iPhone 5s For WebKit Development - Runtime: iOS 10.0 (14A233) - DeviceType: iPhone 5s Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 JavaScriptCore 0x000000010d461d64 WTFCrash + 36 (Assertions.cpp:322) 1 com.apple.WebCore 0x000000010fc4298a WTF::Ref<WebCore::Element>::operator=(WTF::Ref<WebCore::Element>&&) + 74 (Ref.h:98) 2 com.apple.WebCore 0x000000010fc42413 WebCore::FTPDirectoryDocumentParser::appendEntry(WTF::String const&, WTF::String const&, WTF::String const&, bool) + 787 (FTPDirectoryDocument.cpp:115) 3 com.apple.WebCore 0x000000010fc42f6b WebCore::FTPDirectoryDocumentParser::parseAndAppendOneLine(WTF::String const&) + 539 (FTPDirectoryDocument.cpp:268) 4 com.apple.WebCore 0x000000010fc44436 WebCore::FTPDirectoryDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&) + 870 (FTPDirectoryDocument.cpp:394) 5 com.apple.WebCore 0x000000010f7a171f WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter&, char const*, unsigned long) + 175 (DecodedDataDocumentParser.cpp:50) 6 com.apple.WebCore 0x000000010f8d9cd9 WebCore::DocumentWriter::addData(char const*, unsigned long) + 185 (DocumentWriter.cpp:235) 7 com.apple.WebCore 0x000000010f894d50 WebCore::DocumentLoader::commitData(char const*, unsigned long) + 1392 (DocumentLoader.cpp:914) 8 com.apple.WebKit 0x0000000104f6dc9f WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int) + 79 (WebFrameLoaderClient.cpp:950) 9 com.apple.WebCore 0x000000010f89790d WebCore::DocumentLoader::commitLoad(char const*, int) + 205 (DocumentLoader.cpp:832) 10 com.apple.WebCore 0x000000010f8981a9 WebCore::DocumentLoader::dataReceived(WebCore::CachedResource*, char const*, int) + 585 (DocumentLoader.cpp:943) 11 com.apple.WebCore 0x000000010f405c28 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int) + 152 (CachedRawResource.cpp:118) 12 com.apple.WebCore 0x000000010f405ab2 WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&) + 194 (CachedRawResource.cpp:70) 13 com.apple.WebCore 0x00000001111d6ce5 WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) + 485 (SubresourceLoader.cpp:322) 14 com.apple.WebCore 0x00000001111d6ae2 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType) + 98 (SubresourceLoader.cpp:298) 15 com.apple.WebKit 0x000000010522ecd9 WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long long) + 505 (WebResourceLoader.cpp:146) 16 com.apple.WebKit 0x0000000105233ddc void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>&&, std::index_sequence<0ul, 1ul>) + 188 (HandleMessage.h:17) 17 com.apple.WebKit 0x0000000105233bb8 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, std::make_index_sequence<2ul> >(std::__1::tuple<IPC::DataReference, long long>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 88 (HandleMessage.h:23) 18 com.apple.WebKit 0x0000000105233093 void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)>(IPC::MessageDecoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 291 (HandleMessage.h:93) 19 com.apple.WebKit 0x0000000105232856 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::MessageDecoder&) + 502 (WebResourceLoaderMessageReceiver.cpp:62) 20 com.apple.WebKit 0x0000000104cc8d1d WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&) + 157 (NetworkProcessConnection.cpp:60) 21 com.apple.WebKit 0x0000000104aa06f3 IPC::Connection::dispatchMessage(IPC::MessageDecoder&) + 51 (Connection.cpp:896) 22 com.apple.WebKit 0x0000000104a96192 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 770 (Connection.cpp:928) 23 com.apple.WebKit 0x0000000104aa0ce0 IPC::Connection::dispatchOneMessage() + 1504 (Connection.cpp:957) 24 com.apple.WebKit 0x0000000104abfded IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10::operator()() const + 29 (Connection.cpp:890) 25 com.apple.WebKit 0x0000000104abfdbd void std::__1::__invoke_void_return_wrapper<void>::__call<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10&>(IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10&&&) + 45 (__functional_base:469) 26 com.apple.WebKit 0x0000000104abfc29 std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10>, void ()>::operator()() + 41 (functional:1437) 27 JavaScriptCore 0x000000010ce96aaa std::__1::function<void ()>::operator()() const + 26 (functional:1817) 28 JavaScriptCore 0x000000010d4a3379 WTF::RunLoop::performWork() + 297 (RunLoop.cpp:106) 29 JavaScriptCore 0x000000010d4a3b34 WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38) 30 com.apple.CoreFoundation 0x0000000107966941 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 31 com.apple.CoreFoundation 0x000000010794b51c __CFRunLoopDoSources0 + 556 32 com.apple.CoreFoundation 0x000000010794aa06 __CFRunLoopRun + 918 33 com.apple.CoreFoundation 0x000000010794a40d CFRunLoopRunSpecific + 285 34 com.apple.Foundation 0x00000001044ac530 -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 274 35 com.apple.Foundation 0x00000001044ac40b -[NSRunLoop(NSRunLoop) run] + 76 36 libxpc.dylib 0x000000010915d75b _xpc_objc_main + 400 37 libxpc.dylib 0x000000010915fa86 xpc_main + 189 38 com.apple.WebKit.WebContent.Development 0x0000000104412dbc main + 892 (XPCServiceMain.mm:114) 39 libdyld.dylib 0x0000000108e62679 start + 1
(In reply to comment #7) > Comment on attachment 277321 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=277321&action=review > > Thanks Chris for reviewing this patch. > > >> Source/WebCore/ChangeLog:12 > >> + No changes of behavior. > > > > Why no test? > > I don't know if it is possible to make a ftp test. Do you know how? I have just grep'ed for 'ftp' in our layout tests and did not find anything encouraging :/ I guess we don't have layout test support for FTP? > > >>> Source/WebCore/html/FTPDirectoryDocument.cpp:115 > >>> + Ref<Element> nameElement = createTDForFilename(filename); > >> > >> Isn't it OK to assign to a Ref<> after it's been moved out? > > > > Ok, I have just seem the implementation of Ref& operator=(T& object) and it clearly does not allow assignment after a move :/ > > There is an ASSERT in operator= discouraging this usage. Here is the crash > log. > > Process: com.apple.WebKit.WebContent.Development [8919] > Path: > /Users/USER/Documents/*/WebKit.framework/XPCServices/com.apple.WebKit. > WebContent.Development.xpc/com.apple.WebKit.WebContent.Development > Identifier: com.apple.WebKit.WebContent.Development > Version: 602+ (602.1.30+) > Code Type: X86-64 (Native) > Parent Process: launchd_sim.development [6184] > Responsible: com.apple.WebKit.WebContent.Development [8919] > User ID: 501 > > Date/Time: 2016-04-25 19:16:49.008 -0700 > OS Version: Mac OS X 10.11.5 (15F27) > Report Version: 11 > Anonymous UUID: 959E954D-4D93-D4D4-8B62-15433989F34D > > Sleep/Wake UUID: D5FEE3D1-A7DD-4FD0-A49D-825F10DE5647 > > Time Awake Since Boot: 46000 seconds > Time Since Wake: 27000 seconds > > System Integrity Protection: enabled > > Crashed Thread: 0 Dispatch queue: com.apple.main-thread > > Exception Type: EXC_BAD_ACCESS (SIGSEGV) > Exception Codes: KERN_INVALID_ADDRESS at 0x00000000bbadbeef > Exception Note: EXC_CORPSE_NOTIFY > > VM Regions Near 0xbbadbeef: > --> > __TEXT 0000000104412000-0000000104414000 [ 8K] > r-x/rwx SM=COW > /Users/USER/Documents/*/WebKit.framework/XPCServices/com.apple.WebKit. > WebContent.Development.xpc/com.apple.WebKit.WebContent.Development > > Application Specific Information: > CoreSimulator 245 - Device: iPhone 5s For WebKit Development - Runtime: iOS > 10.0 (14A233) - DeviceType: iPhone 5s > > Thread 0 Crashed:: Dispatch queue: com.apple.main-thread > 0 JavaScriptCore 0x000000010d461d64 WTFCrash + 36 > (Assertions.cpp:322) > 1 com.apple.WebCore 0x000000010fc4298a > WTF::Ref<WebCore::Element>::operator=(WTF::Ref<WebCore::Element>&&) + 74 > (Ref.h:98) > 2 com.apple.WebCore 0x000000010fc42413 > WebCore::FTPDirectoryDocumentParser::appendEntry(WTF::String const&, > WTF::String const&, WTF::String const&, bool) + 787 > (FTPDirectoryDocument.cpp:115) > 3 com.apple.WebCore 0x000000010fc42f6b > WebCore::FTPDirectoryDocumentParser::parseAndAppendOneLine(WTF::String > const&) + 539 (FTPDirectoryDocument.cpp:268) > 4 com.apple.WebCore 0x000000010fc44436 > WebCore::FTPDirectoryDocumentParser::append(WTF::RefPtr<WTF::StringImpl>&&) > + 870 (FTPDirectoryDocument.cpp:394) > 5 com.apple.WebCore 0x000000010f7a171f > WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter&, > char const*, unsigned long) + 175 (DecodedDataDocumentParser.cpp:50) > 6 com.apple.WebCore 0x000000010f8d9cd9 > WebCore::DocumentWriter::addData(char const*, unsigned long) + 185 > (DocumentWriter.cpp:235) > 7 com.apple.WebCore 0x000000010f894d50 > WebCore::DocumentLoader::commitData(char const*, unsigned long) + 1392 > (DocumentLoader.cpp:914) > 8 com.apple.WebKit 0x0000000104f6dc9f > WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char > const*, int) + 79 (WebFrameLoaderClient.cpp:950) > 9 com.apple.WebCore 0x000000010f89790d > WebCore::DocumentLoader::commitLoad(char const*, int) + 205 > (DocumentLoader.cpp:832) > 10 com.apple.WebCore 0x000000010f8981a9 > WebCore::DocumentLoader::dataReceived(WebCore::CachedResource*, char const*, > int) + 585 (DocumentLoader.cpp:943) > 11 com.apple.WebCore 0x000000010f405c28 > WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, > unsigned int) + 152 (CachedRawResource.cpp:118) > 12 com.apple.WebCore 0x000000010f405ab2 > WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&) + 194 > (CachedRawResource.cpp:70) > 13 com.apple.WebCore 0x00000001111d6ce5 > WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, > WTF::PassRefPtr<WebCore::SharedBuffer>, long long, WebCore::DataPayloadType) > + 485 (SubresourceLoader.cpp:322) > 14 com.apple.WebCore 0x00000001111d6ae2 > WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long > long, WebCore::DataPayloadType) + 98 (SubresourceLoader.cpp:298) > 15 com.apple.WebKit 0x000000010522ecd9 > WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long > long) + 505 (WebResourceLoader.cpp:146) > 16 com.apple.WebKit 0x0000000105233ddc void > IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), > std::__1::tuple<IPC::DataReference, long long>, 0ul, > 1ul>(WebKit::WebResourceLoader*, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), > std::__1::tuple<IPC::DataReference, long long>&&, std::index_sequence<0ul, > 1ul>) + 188 (HandleMessage.h:17) > 17 com.apple.WebKit 0x0000000105233bb8 void > IPC::callMemberFunction<WebKit::WebResourceLoader, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), > std::__1::tuple<IPC::DataReference, long long>, > std::make_index_sequence<2ul> >(std::__1::tuple<IPC::DataReference, long > long>&&, WebKit::WebResourceLoader*, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 88 > (HandleMessage.h:23) > 18 com.apple.WebKit 0x0000000105233093 void > IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, > WebKit::WebResourceLoader, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long > long)>(IPC::MessageDecoder&, WebKit::WebResourceLoader*, void > (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)) + 291 > (HandleMessage.h:93) > 19 com.apple.WebKit 0x0000000105232856 > WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC:: > Connection&, IPC::MessageDecoder&) + 502 > (WebResourceLoaderMessageReceiver.cpp:62) > 20 com.apple.WebKit 0x0000000104cc8d1d > WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, > IPC::MessageDecoder&) + 157 (NetworkProcessConnection.cpp:60) > 21 com.apple.WebKit 0x0000000104aa06f3 > IPC::Connection::dispatchMessage(IPC::MessageDecoder&) + 51 > (Connection.cpp:896) > 22 com.apple.WebKit 0x0000000104a96192 > IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, > std::__1::default_delete<IPC::MessageDecoder> >) + 770 (Connection.cpp:928) > 23 com.apple.WebKit 0x0000000104aa0ce0 > IPC::Connection::dispatchOneMessage() + 1504 (Connection.cpp:957) > 24 com.apple.WebKit 0x0000000104abfded > IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC:: > MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> > >)::$_10::operator()() const + 29 (Connection.cpp:890) > 25 com.apple.WebKit 0x0000000104abfdbd void > std::__1::__invoke_void_return_wrapper<void>::__call<IPC::Connection:: > enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, > std::__1::default_delete<IPC::MessageDecoder> > >)::$_10&>(IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC:: > MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_10&&&) + > 45 (__functional_base:469) > 26 com.apple.WebKit 0x0000000104abfc29 > std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std:: > __1::unique_ptr<IPC::MessageDecoder, > std::__1::default_delete<IPC::MessageDecoder> >)::$_10, > std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1:: > unique_ptr<IPC::MessageDecoder, > std::__1::default_delete<IPC::MessageDecoder> >)::$_10>, void > ()>::operator()() + 41 (functional:1437) > 27 JavaScriptCore 0x000000010ce96aaa > std::__1::function<void ()>::operator()() const + 26 (functional:1817) > 28 JavaScriptCore 0x000000010d4a3379 > WTF::RunLoop::performWork() + 297 (RunLoop.cpp:106) > 29 JavaScriptCore 0x000000010d4a3b34 > WTF::RunLoop::performWork(void*) + 36 (RunLoopCF.cpp:38) > 30 com.apple.CoreFoundation 0x0000000107966941 > __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 > 31 com.apple.CoreFoundation 0x000000010794b51c __CFRunLoopDoSources0 > + 556 > 32 com.apple.CoreFoundation 0x000000010794aa06 __CFRunLoopRun + 918 > 33 com.apple.CoreFoundation 0x000000010794a40d CFRunLoopRunSpecific + > 285 > 34 com.apple.Foundation 0x00000001044ac530 -[NSRunLoop(NSRunLoop) > runMode:beforeDate:] + 274 > 35 com.apple.Foundation 0x00000001044ac40b -[NSRunLoop(NSRunLoop) > run] + 76 > 36 libxpc.dylib 0x000000010915d75b _xpc_objc_main + 400 > 37 libxpc.dylib 0x000000010915fa86 xpc_main + 189 > 38 com.apple.WebKit.WebContent.Development 0x0000000104412dbc main + 892 > (XPCServiceMain.mm:114) > 39 libdyld.dylib 0x0000000108e62679 start + 1 Even in release, this would crash since it called m_ptr->deref(); and m_ptr has been nulled out by the move.
Comment on attachment 277321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=277321&action=review >>>> Source/WebCore/ChangeLog:12 >>>> + No changes of behavior. >>> >>> Why no test? >> >> I don't know if it is possible to make a ftp test. Do you know how? > > I have just grep'ed for 'ftp' in our layout tests and did not find anything encouraging :/ I guess we don't have layout test support for FTP? Just drop this "No changes of behavior." as this is not true. You are fixing a crash.
Created attachment 277325 [details] Patch
Created attachment 277326 [details] Patch
Comment on attachment 277326 [details] Patch Clearing flags on attachment: 277326 Committed r200074: <http://trac.webkit.org/changeset/200074>
All reviewed patches have been landed. Closing bug.