Index: /trunk/include/VBox/HostServices/VBoxClipboardSvc.h =================================================================== --- /trunk/include/VBox/HostServices/VBoxClipboardSvc.h (revision 82499) +++ /trunk/include/VBox/HostServices/VBoxClipboardSvc.h (revision 82500) @@ -286,8 +286,18 @@ /** Reads data in specified format from the host. * + * This function takes three parameters, a 32-bit format bit, a buffer + * and 32-bit number of bytes read (output). + * + * There was a period during 6.1 development where it would take five parameters + * when VBOX_SHCL_GF_0_CONTEXT_ID was reported by the guest. A 64-bit context + * ID (ignored as purpose undefined), a 32-bit unused flag (MBZ), then the + * 32-bit format bits, number of bytes read (output), and the buffer. This + * format is still accepted. + * * @retval VINF_SUCCESS on success. * @retval VINF_BUFFER_OVERLFLOW (VBox >= 6.1 only) if not enough buffer space - * has been given to retrieve the actual data. The call then must be - * repeated with a buffer size returned from the host in cbData. + * has been given to retrieve the actual data, no data actually copied. + * The call then must be repeated with a buffer size returned from the + * host in cbData. * @retval VERR_INVALID_CLIENT_ID * @retval VERR_WRONG_PARAMETER_COUNT @@ -689,45 +699,20 @@ #define VBOX_SHCL_CPARMS_READ_DATA_REQ 4 -/** - * Reads clipboard data. - */ -typedef struct _VBoxShClReadDataMsg -{ - VBGLIOCHGCMCALL hdr; - - union - { - struct - { - /** uint32_t, out: Requested format. */ - HGCMFunctionParameter format; /* IN uint32_t */ - /** ptr, out: The data buffer. */ - HGCMFunctionParameter ptr; /* IN linear pointer. */ - /** uint32_t, out: Size of returned data, if > ptr->cb, then no data was - * actually transferred and the guest must repeat the call. - */ - HGCMFunctionParameter size; /* OUT uint32_t */ - } v0; - struct - { - /** uint64_t, out: Context ID. */ - HGCMFunctionParameter uContext; - /** uint32_t, out: Read flags; currently unused and must be set to 0. */ - HGCMFunctionParameter fFlags; - /** uint32_t, out: Requested format. */ - HGCMFunctionParameter uFormat; - /** uint32_t, in/out: - * On input: How much data to read max. - * On output: Size of returned data, if > ptr->cb, then no data was - * actually transferred and the guest must repeat the call. - */ - HGCMFunctionParameter cbData; - /** ptr, out: The data buffer. */ - HGCMFunctionParameter pvData; - } v1; - } u; -} VBoxShClReadDataMsg; - -#define VBOX_SHCL_CPARMS_READ_DATA 5 +/** @name VBOX_SHCL_GUEST_FN_DATA_READ + * @{ */ +/** VBOX_SHCL_GUEST_FN_DATA_READ parameters. */ +typedef struct VBoxShClParmDataRead +{ + /** uint32_t, in: Requested format. */ + HGCMFunctionParameter f32Format; + /** ptr, out: The data buffer to put the data in on success. */ + HGCMFunctionParameter pData; + /** uint32_t, out: Size of returned data, if larger than the buffer, then no + * data was actually transferred and the guest must repeat the call. */ + HGCMFunctionParameter cb32Needed; +} VBoxShClParmDataRead; +#define VBOX_SHCL_CPARMS_DATA_READ 3 /**< The parameter count for VBOX_SHCL_GUEST_FN_DATA_READ. */ +#define VBOX_SHCL_CPARMS_DATA_READ_61B 5 /**< The 6.1 dev cycle variant, see VBOX_SHCL_GUEST_FN_DATA_READ. */ +/** @} */ /** Index: /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp =================================================================== --- /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp (revision 82499) +++ /trunk/src/VBox/Additions/common/VBoxGuest/lib/VBoxGuestR3LibClipboard.cpp (revision 82500) @@ -305,19 +305,22 @@ uint32_t *pcbRead) { - VBoxShClReadDataMsg Msg; - LogFlowFuncEnter(); - VBGL_HGCM_HDR_INIT(&Msg.hdr, idClient, VBOX_SHCL_GUEST_FN_DATA_READ, 3); - - VbglHGCMParmUInt32Set(&Msg.u.v0.format, fFormat); - VbglHGCMParmPtrSet(&Msg.u.v0.ptr, pvData, cbData); - VbglHGCMParmUInt32Set(&Msg.u.v0.size, 0); - - int rc = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg.hdr) + sizeof(Msg.u.v0)); + struct + { + VBGLIOCHGCMCALL Hdr; + VBoxShClParmDataRead Parms; + } Msg; + + VBGL_HGCM_HDR_INIT(&Msg.Hdr, idClient, VBOX_SHCL_GUEST_FN_DATA_READ, VBOX_SHCL_CPARMS_DATA_READ); + VbglHGCMParmUInt32Set(&Msg.Parms.f32Format, fFormat); + VbglHGCMParmPtrSet( &Msg.Parms.pData, pvData, cbData); + VbglHGCMParmUInt32Set(&Msg.Parms.cb32Needed, 0); + + int rc = VbglR3HGCMCall(&Msg.Hdr, sizeof(Msg)); if (RT_SUCCESS(rc)) { uint32_t cbRead; - rc = VbglHGCMParmUInt32Get(&Msg.u.v0.size, &cbRead); + rc = VbglHGCMParmUInt32Get(&Msg.Parms.cb32Needed, &cbRead); if (RT_SUCCESS(rc)) { @@ -349,44 +352,5 @@ AssertPtrReturn(pCtx, VERR_INVALID_POINTER); AssertPtrReturn(pData, VERR_INVALID_POINTER); - - int rc; - - LogFlowFuncEnter(); - - if (pCtx->fUseLegacyProtocol) - { - rc = VbglR3ClipboardReadData(pCtx->uClientID, pData->uFormat, pData->pvData, pData->cbData, pcbRead); - } - else - { - VBoxShClReadDataMsg Msg; - - VBGL_HGCM_HDR_INIT(&Msg.hdr, pCtx->uClientID, VBOX_SHCL_GUEST_FN_DATA_READ, VBOX_SHCL_CPARMS_READ_DATA); - - VbglHGCMParmUInt64Set(&Msg.u.v1.uContext, pCtx->uContextID); - VbglHGCMParmUInt32Set(&Msg.u.v1.fFlags, 0); - VbglHGCMParmUInt32Set(&Msg.u.v1.uFormat, pData->uFormat); - VbglHGCMParmUInt32Set(&Msg.u.v1.cbData, pData->cbData); - VbglHGCMParmPtrSet(&Msg.u.v1.pvData, pData->pvData, pData->cbData); - - rc = VbglR3HGCMCall(&Msg.hdr, sizeof(Msg.hdr) + sizeof(Msg.u.v1)); - if (RT_SUCCESS(rc)) - { - uint32_t cbRead; - rc = VbglHGCMParmUInt32Get(&Msg.u.v1.cbData, &cbRead); - if (RT_SUCCESS(rc)) - { - LogFlowFunc(("cbRead=%RU32\n", cbRead)); - - if (cbRead > pData->cbData) - rc = VINF_BUFFER_OVERFLOW; - - *pcbRead = cbRead; - } - } - } - - LogFlowFuncLeaveRC(rc); - return rc; + return VbglR3ClipboardReadData(pCtx->uClientID, pData->uFormat, pData->pvData, pData->cbData, pcbRead); } Index: /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp =================================================================== --- /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (revision 82499) +++ /trunk/src/VBox/HostServices/SharedClipboard/VBoxSharedClipboardSvc.cpp (revision 82500) @@ -1290,162 +1290,175 @@ } -int shClSvcGetDataRead(PSHCLCLIENT pClient, uint32_t cParms, VBOXHGCMSVCPARM paParms[]) +/** + * Handles the VBOX_SHCL_GUEST_FN_DATA_READ message from the guest. + */ +static int shClSvcGetDataRead(PSHCLCLIENT pClient, uint32_t cParms, VBOXHGCMSVCPARM paParms[]) { LogFlowFuncEnter(); - if ( ShClSvcGetMode() != VBOX_SHCL_MODE_HOST_TO_GUEST - && ShClSvcGetMode() != VBOX_SHCL_MODE_BIDIRECTIONAL) - { + /* + * Check if the service mode allows this operation and whether the guest is + * supposed to be reading from the host. + */ + uint32_t uMode = ShClSvcGetMode(); + if ( uMode == VBOX_SHCL_MODE_BIDIRECTIONAL + || uMode == VBOX_SHCL_MODE_HOST_TO_GUEST) + { /* likely */ } + else return VERR_ACCESS_DENIED; - } - - /* Is the guest supposed to read any clipboard data from the host? */ - if (!(pClient->State.fFlags & SHCLCLIENTSTATE_FLAGS_READ_ACTIVE)) - return VERR_WRONG_ORDER; - - int rc; - + + /// @todo r=bird: The management of the SHCLCLIENTSTATE_FLAGS_READ_ACTIVE + /// makes it impossible for the guest to retrieve more than one format from + /// the clipboard. I.e. it can either get the TEXT or the HTML rendering, + /// but not both. So, I've disable the check. */ + //ASSERT_GUEST_RETURN(pClient->State.fFlags & SHCLCLIENTSTATE_FLAGS_READ_ACTIVE, VERR_WRONG_ORDER); + + /* + * Digest parameters. + * + * We are dragging some legacy here from the 6.1 dev cycle, a 5 parameter + * variant which prepends a 64-bit context ID (RAZ as meaning not defined), + * a 32-bit flag (MBZ, no defined meaning) and switches the last two parameters. + */ + ASSERT_GUEST_RETURN( cParms == VBOX_SHCL_CPARMS_DATA_READ + || ( cParms == VBOX_SHCL_CPARMS_DATA_READ_61B + && (pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)), + VERR_WRONG_PARAMETER_COUNT); + + uintptr_t iParm = 0; SHCLCLIENTCMDCTX cmdCtx; RT_ZERO(cmdCtx); + if (cParms == VBOX_SHCL_CPARMS_DATA_READ_61B) + { + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_64BIT, VERR_WRONG_PARAMETER_TYPE); + /* This has no defined meaning and was never used, however the guest passed stuff, so ignore it and leave idContext=0. */ + iParm++; + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); + ASSERT_GUEST_RETURN(paParms[iParm].u.uint32 == 0, VERR_INVALID_FLAGS); + iParm++; + } SHCLDATABLOCK dataBlock; - RT_ZERO(dataBlock); - - if (!(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)) /* Legacy, Guest Additions < 6.1. */ - { - if (cParms != 3) - { - rc = VERR_INVALID_PARAMETER; - } - else - { - rc = HGCMSvcGetU32(&paParms[0], &dataBlock.uFormat); - if (RT_SUCCESS(rc)) - { - if (pClient->State.POD.uFormat == VBOX_SHCL_FMT_NONE) - pClient->State.POD.uFormat = dataBlock.uFormat; - - if (dataBlock.uFormat != pClient->State.POD.uFormat) - { - LogFlowFunc(("Invalid format (pClient->State.POD.uFormat=%RU32 vs dataBlock.uFormat=%RU32)\n", - pClient->State.POD.uFormat, dataBlock.uFormat)); - - rc = VERR_INVALID_PARAMETER; - } - else - { - rc = HGCMSvcGetBuf(&paParms[1], &dataBlock.pvData, &dataBlock.cbData); - } - } - } + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); + dataBlock.uFormat = paParms[iParm].u.uint32; + iParm++; + if (cParms != VBOX_SHCL_CPARMS_DATA_READ_61B) + { + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_PTR, VERR_WRONG_PARAMETER_TYPE); /* Data buffer */ + dataBlock.pvData = paParms[iParm].u.pointer.addr; + dataBlock.cbData = paParms[iParm].u.pointer.size; + iParm++; + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); /*cbDataReturned*/ + iParm++; } else { - if (cParms < VBOX_SHCL_CPARMS_READ_DATA) - { - rc = VERR_INVALID_PARAMETER; - } - else - { - /** @todo Handle paParms[1] flags. */ - - rc = HGCMSvcGetU32(&paParms[2], &dataBlock.uFormat); - if (RT_SUCCESS(rc)) - { - uint32_t cbData; - rc = HGCMSvcGetU32(&paParms[3], &cbData); - if (RT_SUCCESS(rc)) - { - rc = HGCMSvcGetBuf(&paParms[4], &dataBlock.pvData, &dataBlock.cbData); - if (RT_SUCCESS(rc)) - { - if (cbData != dataBlock.cbData) - { - LogFlowFunc(("Invalid data (cbData=%RU32 vs dataBlock.cbData=%RU32)\n", cbData, dataBlock.cbData)); - rc = VERR_INVALID_PARAMETER; - } - } - } - } - } - } + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_32BIT, VERR_WRONG_PARAMETER_TYPE); /*cbDataReturned*/ + iParm++; + ASSERT_GUEST_RETURN(paParms[iParm].type == VBOX_HGCM_SVC_PARM_PTR, VERR_WRONG_PARAMETER_TYPE); /* Data buffer */ + dataBlock.pvData = paParms[iParm].u.pointer.addr; + dataBlock.cbData = paParms[iParm].u.pointer.size; + iParm++; + } + Assert(iParm == cParms); + + /* + * For some reason we need to do this (makes absolutely no sense to bird). + */ + /** @todo r=bird: I really don't get why you need the State.POD.uFormat + * member. I'm sure there is a reason. Incomplete code? */ + if (!(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)) + { + if (pClient->State.POD.uFormat == VBOX_SHCL_FMT_NONE) + pClient->State.POD.uFormat = dataBlock.uFormat; + /// @todo r=bird: This actively breaks copying different types of data into the + /// guest (first copy a text snippet, then you cannot copy any bitmaps), so I've + /// disabled it. + //ASSERT_GUEST_MSG_RETURN(pClient->State.POD.uFormat == dataBlock.uFormat, + // ("Requested %#x, POD.uFormat=%#x\n", dataBlock.uFormat, pClient->State.POD.uFormat), + // VERR_BAD_EXE_FORMAT /*VERR_INTERNAL_ERROR*/); + } + + /* + * Do the reading. + */ + int rc = VINF_SUCCESS; + uint32_t cbActual = 0; + + /* If there is a service extension active, try reading data from it first. */ + if (g_ExtState.pfnExtension) + { + SHCLEXTPARMS parms; + RT_ZERO(parms); + + parms.uFormat = dataBlock.uFormat; + parms.u.pvData = dataBlock.pvData; + parms.cbData = dataBlock.cbData; + + g_ExtState.fReadingData = true; + + /* Read clipboard data from the extension. */ + rc = g_ExtState.pfnExtension(g_ExtState.pvExtension, VBOX_CLIPBOARD_EXT_FN_DATA_READ, &parms, sizeof(parms)); + LogRelFlowFunc(("g_ExtState.fDelayedAnnouncement=%RTbool, g_ExtState.uDelayedFormats=0x%x\n", + g_ExtState.fDelayedAnnouncement, g_ExtState.uDelayedFormats)); + + /* Did the extension send the clipboard formats yet? + * Otherwise, do this now. */ + if (g_ExtState.fDelayedAnnouncement) + { + SHCLFORMATDATA FormatData; + FormatData.fFlags = 0; + FormatData.Formats = g_ExtState.uDelayedFormats; + Assert(FormatData.Formats != VBOX_SHCL_FMT_NONE); /* There better is *any* format here now. */ + + int rc2 = ShClSvcFormatsReport(pClient, &FormatData); + AssertRC(rc2); + + g_ExtState.fDelayedAnnouncement = false; + g_ExtState.uDelayedFormats = 0; + } + + g_ExtState.fReadingData = false; + + if (RT_SUCCESS(rc)) + cbActual = parms.cbData; + } + + /* Note: The host clipboard *always* has precedence over the service extension above, + * so data which has been read above might get overridden by the host clipboard eventually. */ + + /** @todo r=bird: This precedency stuff changed with the 6.1 overhaul and I'm + * not quite sure this makes sense. Imagine you think about connecting + * to a VM running in a non-headless process and there is some stuff in + * the host clipboard preventing you from copy & pasting via the RDP + * client. I guess this means clipboard sharing over RDP is only + * possible when using VBoxHeadless? + * + * Also, looking at the code, I think the host will _always_ return data + * here. Need to test. Sigh. */ if (RT_SUCCESS(rc)) { - uint32_t cbActual = 0; - - /* If there is a service extension active, try reading data from it first. */ - if (g_ExtState.pfnExtension) - { - SHCLEXTPARMS parms; - RT_ZERO(parms); - - parms.uFormat = pClient->State.POD.uFormat; - parms.u.pvData = dataBlock.pvData; - parms.cbData = dataBlock.cbData; - - g_ExtState.fReadingData = true; - - /* Read clipboard data from the extension. */ - rc = g_ExtState.pfnExtension(g_ExtState.pvExtension, VBOX_CLIPBOARD_EXT_FN_DATA_READ, - &parms, sizeof(parms)); - - LogFlowFunc(("g_ExtState.fDelayedAnnouncement=%RTbool, g_ExtState.uDelayedFormats=0x%x\n", - g_ExtState.fDelayedAnnouncement, g_ExtState.uDelayedFormats)); - - /* Did the extension send the clipboard formats yet? - * Otherwise, do this now. */ - if (g_ExtState.fDelayedAnnouncement) - { - SHCLFORMATDATA formatData; - RT_ZERO(formatData); - - formatData.Formats = g_ExtState.uDelayedFormats; - Assert(formatData.Formats != VBOX_SHCL_FMT_NONE); /* There better is *any* format here now. */ - - int rc2 = ShClSvcFormatsReport(pClient, &formatData); - AssertRC(rc2); - - g_ExtState.fDelayedAnnouncement = false; - g_ExtState.uDelayedFormats = 0; - } - - g_ExtState.fReadingData = false; - - if (RT_SUCCESS(rc)) - { - cbActual = parms.cbData; - } - } - - /* Note: The host clipboard *always* has precedence over the service extension above, - * so data which has been read above might get overridden by the host clipboard eventually. */ - + rc = ShClSvcImplReadData(pClient, &cmdCtx, &dataBlock, &cbActual); if (RT_SUCCESS(rc)) { - rc = ShClSvcImplReadData(pClient, &cmdCtx, &dataBlock, &cbActual); - if (RT_SUCCESS(rc)) - { - LogFlowFunc(("cbData=%RU32, cbActual=%RU32\n", dataBlock.cbData, cbActual)); - - /* Return the actual size required to fullfil the request. */ - if (!(pClient->State.fGuestFeatures0 & VBOX_SHCL_GF_0_CONTEXT_ID)) /* Legacy, Guest Additions < 6.1. */ - { - HGCMSvcSetU32(&paParms[2], cbActual); - } - else - { - HGCMSvcSetU32(&paParms[3], cbActual); - } - - /* If the data to return exceeds the buffer the guest supplies, tell it (and let it try again). */ - if (cbActual >= dataBlock.cbData) - rc = VINF_BUFFER_OVERFLOW; - - if (rc == VINF_SUCCESS) - { - /* Only remove "read active" flag after successful read again. */ - pClient->State.fFlags &= ~SHCLCLIENTSTATE_FLAGS_READ_ACTIVE; - } + LogFlowFunc(("cbData=%RU32, cbActual=%RU32\n", dataBlock.cbData, cbActual)); + + /* Return the actual size required to fullfil the request. */ + if (cParms != VBOX_SHCL_CPARMS_DATA_READ_61B) + HGCMSvcSetU32(&paParms[2], cbActual); + else + HGCMSvcSetU32(&paParms[3], cbActual); + + /* If the data to return exceeds the buffer the guest supplies, tell it (and let it try again). */ + if (cbActual >= dataBlock.cbData) + rc = VINF_BUFFER_OVERFLOW; + + if (rc == VINF_SUCCESS) + { + /* Only remove "read active" flag after successful read again. */ + /** @todo r=bird: This doesn't make any effing sense. What if the guest + * wants to read another format??? */ + pClient->State.fFlags &= ~SHCLCLIENTSTATE_FLAGS_READ_ACTIVE; } } @@ -1872,8 +1885,6 @@ case VBOX_SHCL_GUEST_FN_DATA_READ: - { rc = shClSvcGetDataRead(pClient, cParms, paParms); break; - } case VBOX_SHCL_GUEST_FN_DATA_WRITE: