Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/shellextension #7297

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions shell_integration/windows/NCContextMenu/NCClientInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#include "NCClientInterface.h"

#include "CommunicationSocket.h"

Check failure on line 17 in shell_integration/windows/NCContextMenu/NCClientInterface.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.cpp:17:10 [clang-diagnostic-error]

'CommunicationSocket.h' file not found
#include "StringUtil.h"

#include <shlobj.h>
Expand All @@ -26,45 +26,56 @@
#include <sstream>
#include <iterator>
#include <unordered_set>
#include <locale>
#include <codecvt>

using namespace std;

#define PIPE_TIMEOUT 5*1000 //ms

NCClientInterface::ContextMenuInfo NCClientInterface::FetchInfo(const std::wstring &files)
NCClientInterface::ContextMenuInfo NCClientInterface::FetchInfo(const std::wstring &files, std::ofstream &logger)

Check warning on line 36 in shell_integration/windows/NCContextMenu/NCClientInterface.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.cpp:36:55 [modernize-use-trailing-return-type]

use a trailing return type for this function

Check warning on line 36 in shell_integration/windows/NCContextMenu/NCClientInterface.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.cpp:36:55 [readability-function-cognitive-complexity]

function 'FetchInfo' has cognitive complexity of 29 (threshold 25)
{
auto pipename = CommunicationSocket::DefaultPipePath();

CommunicationSocket socket;
if (!WaitNamedPipe(pipename.data(), PIPE_TIMEOUT)) {
logger << "error with WaitNamedPipe" << std::endl;
return {};
}
if (!socket.Connect(pipename)) {
logger << "error with Connect" << std::endl;
return {};
}
socket.SendMsg(L"GET_STRINGS:CONTEXT_MENU_TITLE\n");
socket.SendMsg((L"GET_MENU_ITEMS:" + files + L"\n").data());
socket.SendMsg(L"GET_STRINGS:CONTEXT_MENU_TITLE\n", logger);
socket.SendMsg((L"GET_MENU_ITEMS:" + files + L"\n").data(), logger);

ContextMenuInfo info;
std::wstring response;
int sleptCount = 0;

constexpr auto noReplyTimeout = 20;
constexpr auto replyTimeout = 200;
bool receivedReplyFromDesktopClient = false;
while ((!receivedReplyFromDesktopClient && sleptCount < noReplyTimeout) || (receivedReplyFromDesktopClient && sleptCount < replyTimeout)) {
if (socket.ReadLine(&response)) {
logger << "trying to read a line" << std::endl;

if (socket.ReadLine(&response, logger)) {
logger << "received: " << StringUtil::toUtf8(response.c_str()) << std::endl;
if (StringUtil::begins_with(response, wstring(L"REGISTER_PATH:"))) {
logger << "received: REGISTER_PATH" << std::endl;
wstring responsePath = response.substr(14); // length of REGISTER_PATH
info.watchedDirectories.push_back(responsePath);
}
else if (StringUtil::begins_with(response, wstring(L"STRING:"))) {
logger << "received: STRING" << std::endl;
wstring stringName, stringValue;

Check warning on line 71 in shell_integration/windows/NCContextMenu/NCClientInterface.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.cpp:71:17 [readability-isolate-declaration]

multiple declarations in a single statement reduces readability
if (!StringUtil::extractChunks(response, stringName, stringValue))
continue;
if (stringName == L"CONTEXT_MENU_TITLE")
info.contextMenuTitle = move(stringValue);
} else if (StringUtil::begins_with(response, wstring(L"MENU_ITEM:"))) {
logger << "received: MENU_ITEM" << std::endl;
wstring commandName, flags, title;

Check warning on line 78 in shell_integration/windows/NCContextMenu/NCClientInterface.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.cpp:78:17 [readability-isolate-declaration]

multiple declarations in a single statement reduces readability
if (!StringUtil::extractChunks(response, commandName, flags, title))
continue;
info.menuItems.push_back({ commandName, flags, title });
Expand All @@ -72,18 +83,22 @@
receivedReplyFromDesktopClient = true;
continue;
} else if (StringUtil::begins_with(response, wstring(L"GET_MENU_ITEMS:END"))) {
logger << "received: GET_MENU_ITEMS:END" << std::endl;
break; // Stop once we completely received the last sent request
} else {
logger << "received: another reply" << std::endl;
}
}
else {
} else {
logger << "received nothing" << std::endl;
Sleep(50);
++sleptCount;
}
}

return info;
}

void NCClientInterface::SendRequest(const wchar_t *verb, const std::wstring &path)
void NCClientInterface::SendRequest(const wchar_t *verb, const std::wstring &path, std::ofstream &logger)
{
auto pipename = CommunicationSocket::DefaultPipePath();

Expand All @@ -95,5 +110,5 @@
return;
}

socket.SendMsg((verb + (L":" + path + L"\n")).data());
socket.SendMsg((verb + (L":" + path + L"\n")).data(), logger);
}
5 changes: 3 additions & 2 deletions shell_integration/windows/NCContextMenu/NCClientInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

#pragma once

#include <string>

Check failure on line 32 in shell_integration/windows/NCContextMenu/NCClientInterface.h

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCClientInterface.h:32:10 [clang-diagnostic-error]

'string' file not found
#include <vector>
#include <unordered_map>
#include <queue>
Expand All @@ -37,6 +37,7 @@
#include <mutex>
#include <atomic>
#include <condition_variable>
#include <fstream>

class CommunicationSocket;

Expand All @@ -52,8 +53,8 @@
};
std::vector<MenuItem> menuItems;
};
static ContextMenuInfo FetchInfo(const std::wstring &files);
static void SendRequest(const wchar_t *verb, const std::wstring &path);
static ContextMenuInfo FetchInfo(const std::wstring &files, std::ofstream &logger);
static void SendRequest(const wchar_t *verb, const std::wstring &path, std::ofstream &logger);
};

#endif //ABSTRACTSOCKETHANDLER_H
20 changes: 18 additions & 2 deletions shell_integration/windows/NCContextMenu/NCContextMenu.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/**

Check notice on line 1 in shell_integration/windows/NCContextMenu/NCContextMenu.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on shell_integration/windows/NCContextMenu/NCContextMenu.cpp

File shell_integration/windows/NCContextMenu/NCContextMenu.cpp does not conform to Custom style guidelines. (lines 24, 151, 155, 169)
* Copyright (c) 2015 Daniel Molkentin <[email protected]>. All rights reserved.
*
* This library is free software; you can redistribute it and/or modify it under
Expand All @@ -21,16 +21,22 @@
#include <StringUtil.h>
#include <strsafe.h>

#include <iostream>
#include <fstream>

extern long g_cDllRef;

Check warning on line 27 in shell_integration/windows/NCContextMenu/NCContextMenu.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCContextMenu/NCContextMenu.cpp:27:13 [cppcoreguidelines-avoid-non-const-global-variables]

variable 'g_cDllRef' is non-const and globally accessible, consider making it const

NCContextMenu::NCContextMenu(void)
: m_cRef(1)
{
InterlockedIncrement(&g_cDllRef);
m_logger.open("c:\\test.log");
m_logger << "hello world" << std::endl;
}

NCContextMenu::~NCContextMenu(void)
{
m_logger << "NCContextMenu::~NCContextMenu" << std::endl;
InterlockedDecrement(&g_cDllRef);
}

Expand All @@ -39,6 +45,7 @@
// Query to the interface the component supported.
IFACEMETHODIMP NCContextMenu::QueryInterface(REFIID riid, void **ppv)
{
m_logger << "NCContextMenu::QueryInterface" << std::endl;
static const QITAB qit[] =
{
QITABENT(NCContextMenu, IContextMenu),
Expand All @@ -51,12 +58,14 @@
// Increase the reference count for an interface on an object.
IFACEMETHODIMP_(ULONG) NCContextMenu::AddRef()
{
m_logger << "NCContextMenu::AddRef" << std::endl;
return InterlockedIncrement(&m_cRef);
}

// Decrease the reference count for an interface on an object.
IFACEMETHODIMP_(ULONG) NCContextMenu::Release()
{
m_logger << "NCContextMenu::Release" << std::endl;
ULONG cRef = InterlockedDecrement(&m_cRef);
if (0 == cRef) {
delete this;
Expand All @@ -74,6 +83,7 @@
IFACEMETHODIMP NCContextMenu::Initialize(
LPCITEMIDLIST, LPDATAOBJECT pDataObj, HKEY)
{
m_logger << "NCContextMenu::Initialize" << std::endl;
m_selectedFiles.clear();

if (!pDataObj) {
Expand Down Expand Up @@ -129,17 +139,20 @@

IFACEMETHODIMP NCContextMenu::QueryContextMenu(HMENU hMenu, UINT indexMenu, UINT idCmdFirst, UINT idCmdLast, UINT uFlags)
{
m_logger << "NCContextMenu::QueryContextMenu" << std::endl;
// If uFlags include CMF_DEFAULTONLY then we should not do anything.
if (CMF_DEFAULTONLY & uFlags)
{
return MAKE_HRESULT(SEVERITY_SUCCESS, 0, USHORT(0));
}

m_info = NCClientInterface::FetchInfo(m_selectedFiles);
m_info = NCClientInterface::FetchInfo(m_selectedFiles, m_logger);
if (m_info.menuItems.empty()) {
m_logger << "NCContextMenu::QueryContextMenu " << "empty info" << std::endl;
return MAKE_HRESULT(SEVERITY_SUCCESS, 0, USHORT(0));
}

m_logger << "NCContextMenu::QueryContextMenu" << "insert separator" << std::endl;
InsertSeperator(hMenu, indexMenu++);

HMENU hSubmenu = CreateMenu();
Expand All @@ -153,6 +166,7 @@
if (!InsertMenuItem(hMenu, indexMenu++, TRUE, &mii))
return HRESULT_FROM_WIN32(GetLastError());
}
m_logger << "NCContextMenu::QueryContextMenu" << "insert separator" << std::endl;
InsertSeperator(hMenu, indexMenu++);

UINT indexSubMenu = 0;
Expand All @@ -179,6 +193,7 @@

IFACEMETHODIMP NCContextMenu::InvokeCommand(LPCMINVOKECOMMANDINFO pici)
{
m_logger << "NCContextMenu::InvokeCommand" << std::endl;
std::wstring command;

CMINVOKECOMMANDINFOEX *piciEx = nullptr;
Expand Down Expand Up @@ -215,13 +230,14 @@
return E_FAIL;
}

NCClientInterface::SendRequest(command.data(), m_selectedFiles);
NCClientInterface::SendRequest(command.data(), m_selectedFiles, m_logger);
return S_OK;
}

IFACEMETHODIMP NCContextMenu::GetCommandString(UINT_PTR idCommand,
UINT uFlags, UINT *pwReserved, LPSTR pszName, UINT cchMax)
{
m_logger << "NCContextMenu::GetCommandString" << std::endl;
if (idCommand < m_info.menuItems.size() && uFlags == GCS_VERBW) {
return StringCchCopyW(reinterpret_cast<PWSTR>(pszName), cchMax,
m_info.menuItems[idCommand].command.data());
Expand Down
8 changes: 7 additions & 1 deletion shell_integration/windows/NCContextMenu/NCContextMenu.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@
#define NCCONTEXTMENU_H

#pragma once
#include "NCClientInterface.h"

#include <shlobj.h> // For IShellExtInit and IContextMenu

#include <string>
#include "NCClientInterface.h"
#include <fstream>
#include <iostream>

class NCContextMenu : public IShellExtInit, public IContextMenu
{
Expand Down Expand Up @@ -48,6 +52,8 @@ class NCContextMenu : public IShellExtInit, public IContextMenu
// The name of the selected files (separated by '\x1e')
std::wstring m_selectedFiles;
NCClientInterface::ContextMenuInfo m_info;

std::ofstream m_logger;
};

#endif //NCCONTEXTMENU_H
15 changes: 11 additions & 4 deletions shell_integration/windows/NCOverlays/NCOverlay.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/**

Check notice on line 1 in shell_integration/windows/NCOverlays/NCOverlay.cpp

View workflow job for this annotation

GitHub Actions / build

Run clang-format on shell_integration/windows/NCOverlays/NCOverlay.cpp

File shell_integration/windows/NCOverlays/NCOverlay.cpp does not conform to Custom style guidelines. (lines 44)
* Copyright (c) 2000-2013 Liferay, Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or modify it under
Expand Down Expand Up @@ -35,22 +35,25 @@

unique_ptr<RemotePathChecker> s_instance;

RemotePathChecker *getGlobalChecker()
RemotePathChecker *getGlobalChecker(ofstream &logger)

Check warning on line 38 in shell_integration/windows/NCOverlays/NCOverlay.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCOverlays/NCOverlay.cpp:38:20 [modernize-use-trailing-return-type]

use a trailing return type for this function
{
// On Vista we'll run into issue #2680 if we try to create the thread+pipe connection
// on any DllGetClassObject of our registered classes.
// Work around the issue by creating the static RemotePathChecker only once actually needed.
static once_flag s_onceFlag;
call_once(s_onceFlag, [] { s_instance.reset(new RemotePathChecker); });
call_once(s_onceFlag, [&logger] { s_instance.reset(new RemotePathChecker{logger}); });

return s_instance.get();
}

}
NCOverlay::NCOverlay(int state)
NCOverlay::NCOverlay(int state)

Check warning on line 50 in shell_integration/windows/NCOverlays/NCOverlay.cpp

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCOverlays/NCOverlay.cpp:50:1 [cppcoreguidelines-pro-type-member-init]

constructor does not initialize these fields: ULONG, m_logger
: _referenceCount(1)
, _state(state)
{
// it is saved under %userprofile%
m_logger.open("C:\\overlay.log");
m_logger << "debug" << std::endl;
}

NCOverlay::~NCOverlay(void)
Expand Down Expand Up @@ -120,10 +123,11 @@

IFACEMETHODIMP NCOverlay::IsMemberOf(PCWSTR pwszPath, DWORD dwAttrib)
{
RemotePathChecker* checker = getGlobalChecker();
auto checker = getGlobalChecker(m_logger);
std::shared_ptr<const std::vector<std::wstring>> watchedDirectories = checker->WatchedDirectories();

if (watchedDirectories->empty()) {
m_logger << "list of watched directories are empty" << std::endl;
return MAKE_HRESULT(S_FALSE, 0, 0);
}

Expand All @@ -136,13 +140,16 @@
}

if (!watched) {
m_logger << "watched is false" << std::endl;
return MAKE_HRESULT(S_FALSE, 0, 0);
}

int state = 0;
if (!checker->IsMonitoredPath(pwszPath, &state)) {
m_logger << "not monitored path: " << pwszPath << std::endl;
return MAKE_HRESULT(S_FALSE, 0, 0);
}

return MAKE_HRESULT(state == _state ? S_OK : S_FALSE, 0, 0);
}

Expand Down
6 changes: 4 additions & 2 deletions shell_integration/windows/NCOverlays/NCOverlay.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@

#pragma once

#include <shlobj.h>

Check failure on line 20 in shell_integration/windows/NCOverlays/NCOverlay.h

View workflow job for this annotation

GitHub Actions / build

shell_integration/windows/NCOverlays/NCOverlay.h:20:10 [clang-diagnostic-error]

'shlobj.h' file not found
#include <fstream>

class NCOverlay : public IShellIconOverlayIdentifier

{
public:
NCOverlay(int state);
explicit NCOverlay(int state);

IFACEMETHODIMP_(ULONG) AddRef();
IFACEMETHODIMP GetOverlayInfo(PWSTR pwszIconFile, int cchMax, int *pIndex, DWORD *pdwFlags);
Expand All @@ -38,6 +39,7 @@
private:
long _referenceCount;
int _state;
std::ofstream m_logger;
};

#endif
#endif
Loading
Loading