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

Fix attachment type (#175) #184

Merged
merged 18 commits into from
Dec 13, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
62 changes: 32 additions & 30 deletions src/puppet-wechat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ import {
import {
WebAppMsgType,
WebContactRawPayload,
WebMediaType,
UploadMediaType,
WebMessageMediaPayload,
WebMessageRawPayload,
WebMessageType,
Expand Down Expand Up @@ -1114,16 +1114,20 @@ export class PuppetWeChat extends PUPPET.Puppet {
await this.memory.save()
}

private extToType (ext: string): WebMessageType {
switch (ext) {
case '.bmp':
case '.jpeg':
case '.jpg':
case '.png':
/**
* `isImg()` @see https://github.com/wechaty/webwx-app-tracker/blob/a12c78fb8bd7186c0f3bb0e18dd611151e6b8aac/formatted/webwxApp.js#L3441-L3450
* `getMsgType()` @see https://github.com/wechaty/webwx-app-tracker/blob/a12c78fb8bd7186c0f3bb0e18dd611151e6b8aac/formatted/webwxApp.js#L3452-L3463
*/
private getMsgType (ext: string): WebMessageType {
switch (ext.toLowerCase()) {
case 'bmp':
case 'jpeg':
case 'jpg':
case 'png':
return WebMessageType.IMAGE
case '.gif':
case 'gif':
return WebMessageType.EMOTICON
case '.mp4':
case 'mp4':
return WebMessageType.VIDEO
default:
return WebMessageType.APP
Expand Down Expand Up @@ -1227,43 +1231,41 @@ export class PuppetWeChat extends PUPPET.Puppet {

}

private getExtName (filename:string) {
return path.extname(filename).slice(1)
}

private async uploadMedia (
file : FileBoxInterface,
toUserName : string,
): Promise<WebMessageMediaPayload> {
const filename = file.name
const ext = path.extname(filename) // message.ext()

// const contentType = Misc.mime(ext)
const ext = this.getExtName(filename)
const msgType = this.getMsgType(ext)
const contentType = mime.getType(ext) || file.mediaType || undefined
// const contentType = message.mimeType()
if (!contentType) {
throw new Error('no MIME Type found on mediaMessage: ' + file.name)
}
let mediatype: WebMediaType

switch (ext) {
case '.bmp':
case '.jpeg':
case '.jpg':
case '.png':
case '.gif':
mediatype = WebMediaType.Image

let mediatype: 'pic'|'video'|'doc'
switch (msgType) {
// case WebMessageType.EMOTICON: //gif cannot be "pic", it will cause sending wrong picture. #178
case WebMessageType.IMAGE:
mediatype = 'pic'
break
case '.mp4':
mediatype = WebMediaType.Video
case WebMessageType.VIDEO:
mediatype = 'video'
break
default:
mediatype = WebMediaType.Attachment
mediatype = 'doc'
}

const buffer = await new Promise<Buffer>((resolve, reject) => {
const bl = new BufferList((err: undefined | Error, data: Buffer) => {
if (err) reject(err)
else resolve(data)
})
// Huan(202110): how to remove any?
file.pipe(bl as any)
file.pipe(bl)
})

// Sending video files is not allowed to exceed 20MB
Expand All @@ -1273,7 +1275,7 @@ export class PuppetWeChat extends PUPPET.Puppet {
const LARGE_FILE_SIZE = 25 * 1024 * 1024
const MAX_VIDEO_SIZE = 20 * 1024 * 1024

if (mediatype === WebMediaType.Video && buffer.length > MAX_VIDEO_SIZE) {
if (msgType === WebMessageType.VIDEO && buffer.length > MAX_VIDEO_SIZE) {
throw new Error(`Sending video files is not allowed to exceed ${MAX_VIDEO_SIZE / 1024 / 1024}MB`)
}
if (buffer.length > MAX_FILE_SIZE) {
Expand Down Expand Up @@ -1311,7 +1313,7 @@ export class PuppetWeChat extends PUPPET.Puppet {
DataLen: size,
FileMd5: fileMd5,
FromUserName: fromUserName,
MediaType: WebMediaType.Attachment,
MediaType: UploadMediaType.Attachment,
Signature: '',
StartPos: 0,
ToUserName: toUserName,
Expand Down Expand Up @@ -1518,7 +1520,7 @@ export class PuppetWeChat extends PUPPET.Puppet {
// console.log('mediaData.MsgType', mediaData.MsgType)
// console.log('rawObj.MsgType', message.rawObj && message.rawObj.MsgType)

mediaData.MsgType = this.extToType(path.extname(file.name))
mediaData.MsgType = this.getMsgType(this.getExtName(file.name))
log.silly('PuppetWeChat', 'sendMedia() destination: %s, mediaId: %s, MsgType; %s)',
conversationId,
mediaData.MediaId,
Expand Down
21 changes: 16 additions & 5 deletions src/web-schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,22 @@ export interface WebMessageRawPayload {

}

export const enum WebMediaType {
Image = 'pic',
Video = 'video',
Attachment = 'doc',
// Audio = 3, useless now
/**
* UploadMediaType from webwx-app
* @see https://github.com/wechaty/webwx-app-tracker/blob/a12c78fb8bd7186c0f3bb0e18dd611151e6b8aac/formatted/webwxApp.js#L7545-L7549
*
* //upload media type
* UPLOAD_MEDIA_TYPE_IMAGE: 1
* UPLOAD_MEDIA_TYPE_VIDEO: 2
* UPLOAD_MEDIA_TYPE_AUDIO: 3
* UPLOAD_MEDIA_TYPE_ATTACHMENT: 4,
*/
export const enum UploadMediaType {
Unknown = 0,
Image = 1,
Video = 2,
Audio = 3,
Attachment = 4,
}

export interface WebRoomRawMember {
Expand Down
178 changes: 178 additions & 0 deletions tests/puppeteer-attachment.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
#!/usr/bin/env -S node --no-warnings --loader ts-node/esm
/**
* Wechaty - https://github.com/chatie/wechaty
*
* @copyright 2016-2018 Huan LI <[email protected]>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
import { test, sinon } from 'tstest'

import { log } from '../src/config.js'

import { PuppetWeChat } from '../src/puppet-wechat.js'
import { WebMessageMediaPayload, WebMessageType } from '../src/web-schemas.js'
import { FileBox } from 'file-box'
import request from 'request'
import { extname } from 'path'

class PuppetTest extends PuppetWeChat {}

test('Send Attachment', async (t) => {
const puppet = new PuppetTest()

const sandbox = sinon.createSandbox()
sandbox.stub(puppet.bridge, 'getCheckUploadUrl').returns(Promise.resolve('getCheckUploadUrl'))
sandbox.stub(puppet.bridge, 'getUploadMediaUrl').returns(Promise.resolve('getUploadMediaUrl'))
sandbox.stub(puppet.bridge, 'getBaseRequest').returns(Promise.resolve('{}'))
sandbox.stub(puppet.bridge, 'getPassticket').returns(Promise.resolve('getPassticket'))
sandbox.stub(puppet.bridge, 'cookies').returns(Promise.resolve([]))
sandbox.stub(puppet.bridge, 'hostname').returns(Promise.resolve('hostname'))
sandbox.replaceGetter(puppet, 'currentUserId', () => 'currentUserId')
const conversationId = 'filehelper'
const uploadMediaUrl = await puppet.bridge.getUploadMediaUrl()
const checkUploadUrl = await puppet.bridge.getCheckUploadUrl()
const mockedResCheckUpload = {
AESKey: 'AESKey',
Signature: 'Signature',
}
const mockedResUploadMedia = {
MediaId: 'MediaId',
}
const getExtName = (filename: string) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we re-define this function, instead of using the puppet.getExtName() for testing?

return extname(filename).slice(1)
}
const extToType = (ext: string): WebMessageType => {
Copy link
Member

@huan huan Dec 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we re-define this function, instead of using the puppet.extToType() for testing?

I think we should test those methods on the puppet, please remove the defines in the tests, and use the existing code in puppet instead.

This PR will be ready to be merged after we finished improving testing codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK,I just want to prevent extToType and getExtName from being accidentally modified

switch (ext.toLowerCase()) {
case 'bmp':
case 'jpeg':
case 'jpg':
case 'png':
return WebMessageType.IMAGE
case 'gif':
return WebMessageType.EMOTICON
case 'mp4':
return WebMessageType.VIDEO
default:
return WebMessageType.APP
}
}
const mockSendMedia = async (msg: WebMessageMediaPayload) => {
log.silly('TestMessage', 'mocked bridge.sendMedia(%o)', msg)
const ext = getExtName(msg.FileName)
const msgType = extToType(ext)
t.match(msg.MMFileExt, /^\w+$/, 'should be WebMessageMediaPayload.MMFileExt matches /^\\w+$/')
t.equal(msg.MsgType, msgType, `should be WebMessageMediaPayload.MsgType is "${msgType}"`)
t.equal(msg.MMFileExt, ext, `should be WebMessageMediaPayload.MMFileExt is "${ext}"`)
return true
}
const mockPostRequest = (
options: request.RequiredUriUrl & request.CoreOptions,
callback?: request.RequestCallback,
): request.Request => {
log.silly('TestMessage', 'mocked request.post(%o)', options)
let path: string | null = null
if ('url' in options) {
if (typeof options.url === 'object') {
path = options.url.path as string
} else {
path = options.url
}
} else if ('uri' in options) {
if (typeof options.uri === 'object') {
path = options.uri.path as string
} else {
path = options.uri
}
}
if (path && callback) {
if (path.includes(uploadMediaUrl)) {
log.silly(
'TestMessage',
'requesting %s:%o',
uploadMediaUrl,
options.formData,
)
const formData = options.formData as {
name: string;
mediatype: string;
type: string;
uploadmediarequest: string;
}
const uploadmediarequest = JSON.parse(formData.uploadmediarequest) as {
AESKey: string;
BaseRequest: any;
ClientMediaId: number;
DataLen: number;
FileMd5: string;
FromUserName: string;
MediaType: number;
Signature: string;
StartPos: number;
ToUserName: string;
TotalLen: number;
UploadType: number;
}
const name = formData.name
const ext = getExtName(name)
let mediatype: string
switch (extToType(ext)) {
case WebMessageType.IMAGE:
mediatype = 'pic'
break
case WebMessageType.VIDEO:
mediatype = 'video'
break
default:
mediatype = 'doc'
}
t.equal(formData.mediatype, mediatype, `should be mediatype is "${mediatype}"`)
t.equal(uploadmediarequest.MediaType, 4, 'should be uploadmediarequest.MediaType is 4')
t.equal(uploadmediarequest.UploadType, 2, 'should be uploadmediarequest.UploadType is 2')

callback(null, {} as any, mockedResUploadMedia)
} else if (path.includes(checkUploadUrl)) {
callback(null, {} as any, mockedResCheckUpload)
} else {
log.silly('Unknown request:%s', path)
}
}
return null as any
}
sandbox.stub(puppet.bridge, 'sendMedia').callsFake(mockSendMedia)
sandbox.stub(request, 'post').callsFake(mockPostRequest)

await Promise.all(
[
'png',
'jpg',
'jpeg',
'bmp',
'gif',
'html',
'txt',
'docx',
'doc',
'xlsx',
'csv',
'mp3',
'mp4',
'mkv',
].map(async (ext) => {
const giffile = FileBox.fromBuffer(Buffer.alloc(10), 'test.' + ext)
await puppet.messageSendFile(conversationId, giffile)
}),
)
sandbox.restore()
})