feat: implement wallpaper cache plugin for dde-services#61
Conversation
There was a problem hiding this comment.
Sorry @yixinshark, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
9aeb6af to
2f800bc
Compare
The wallpapercache plugin provides the following main functions: - Wallpaper scaling: Asynchronously scales wallpapers to match different screen resolutions and caches the results for performance. - Image effects: Synchronously generates processed images (such as Gaussian blur) for the greeter and lock screen. - D-Bus API: Provides a unified interface for desktop components to request processed wallpaper paths, supporting both direct file paths and file descriptors (via FD passing). - Cache management: Automatically manages and cleans up processed image caches to optimize disk usage. Log: implement wallpaper cache plugin for dde-services Pms: TASK-384099 Change-Id: Ie67988592285bdfb9ec541ac849f5bef264852bd
2f800bc to
896f631
Compare
deepin pr auto review代码审查报告1. 语法与逻辑问题1.1 文件路径处理问题位置: QString format = originalFileInfo.suffix(); // TODO format is null ?
// md5_1920x1080.jpg
QString fileName = md5String + "_" + sizeToString(task.targetSize) + "." + format;问题:
建议: QString format = originalFileInfo.suffix();
if (format.isEmpty()) {
// 尝试从 QImageReader 获取格式
QImageReader reader(task.originalPath);
format = reader.format();
if (format.isEmpty()) {
// 默认使用 jpg 格式
format = "jpg";
}
}
QString fileName = md5String + "_" + sizeToString(task.targetSize) + "." + format;1.2 文件描述符处理问题位置: char buffer[4096];
ssize_t bytesRead;
while ((bytesRead = read(fdi, buffer, sizeof(buffer))) > 0) {
if (originFile.write(buffer, bytesRead) != bytesRead) {
qWarning() << "Failed to write data to destination file.";
break;
}
}问题:
建议: char buffer[4096];
ssize_t bytesRead;
while ((bytesRead = read(fdi, buffer, sizeof(buffer))) > 0) {
qint64 bytesWritten = originFile.write(buffer, bytesRead);
if (bytesWritten != bytesRead) {
qWarning() << "Failed to write data to destination file. Expected:"
<< bytesRead << "Written:" << bytesWritten;
originFile.close();
return QString();
}
}
if (bytesRead < 0) {
qWarning() << "Error reading from file descriptor:" << strerror(errno);
originFile.close();
return QString();
}1.3 线程安全问题位置: QStringList CachedWallpaper::getCachedImagePaths(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
QList<QSize> noCachedSizes;
QStringList results;
QString pathMd5 = ScaleImageThread::pathMd5(originalPath, isMd5Path);
if (m_cachedImages.contains(pathMd5)) {
const QMap<QString, QString> &map = m_cachedImages[pathMd5];
// ...
}
// ...
}问题:
建议: QStringList CachedWallpaper::getCachedImagePaths(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
QMutexLocker locker(&m_cacheMutex); // 添加互斥锁保护
QList<QSize> noCachedSizes;
QStringList results;
QString pathMd5 = ScaleImageThread::pathMd5(originalPath, isMd5Path);
if (m_cachedImages.contains(pathMd5)) {
const QMap<QString, QString> &map = m_cachedImages[pathMd5];
// ...
}
// ...
}并在 private:
QMutex m_cacheMutex;2. 代码质量问题2.1 魔法数字和硬编码值位置: #define PIXMIX_MATRIX 16 // Image sample size
#define PIXMIX_OPACITY 90 // Color opacity
#define PIXMIX_SATURATION 50 // Saturation
#define PIXMIX_BRIGHTNESS -60 // Brightness问题:
建议:
2.2 错误处理不一致位置: 多处 问题:
建议:
2.3 缺少输入验证位置: QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
if (!QFile::exists(originalPath)) {
return QStringList() << originalPath;
}
// ...
}问题:
建议: QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
if (!QFile::exists(originalPath)) {
qWarning() << "Original image not exists:" << originalPath;
return QStringList();
}
if (sizeArray.isEmpty()) {
qWarning() << "Size array is empty";
return QStringList() << originalPath;
}
QList<QSize> sizes = parseSizeArray(sizeArray);
for (const QSize &size : sizes) {
if (size.isEmpty() || size.width() <= 0 || size.height() <= 0) {
qWarning() << "Invalid size:" << size;
return QStringList() << originalPath;
}
if (size.width() > 8192 || size.height() > 8192) {
qWarning() << "Size too large:" << size;
return QStringList() << originalPath;
}
}
// ...
}3. 代码性能问题3.1 频繁的文件系统访问位置: QString CachedWallpaper::getBlurImagePath(const QString &originalPath)
{
QString pathMd5 = QCryptographicHash::hash(originalPath.toUtf8(), QCryptographicHash::Md5).toHex();
QMutexLocker locker(&m_blurGenerateMutex);
auto it = m_blurImageCache.constFind(pathMd5);
if (it != m_blurImageCache.constEnd() && QFile::exists(it.value())) {
return it.value();
}
// ...
}问题:
建议:
3.2 图像处理性能位置: QImage ImageEffectProcessor::processPixmixEffect(const QString &imagePath)
{
QImage originalImage;
if (!originalImage.load(imagePath)) {
qWarning() << "Failed to load image:" << imagePath;
return QImage();
}
QColor averageColor = calculateAverageColor(originalImage, PIXMIX_MATRIX);
// ...
}问题:
建议: QImage ImageEffectProcessor::processPixmixEffect(const QString &imagePath)
{
// 先获取图像尺寸,避免加载过大图像
QImageReader reader(imagePath);
QSize originalSize = reader.size();
// 如果图像过大,先进行采样
if (originalSize.width() > 3840 || originalSize.height() > 2160) {
reader.setScaledSize(QSize(1920, 1080)); // 采样到1080p
}
QImage originalImage;
if (!reader.read(&originalImage)) {
qWarning() << "Failed to load image:" << imagePath;
return QImage();
}
QColor averageColor = calculateAverageColor(originalImage, PIXMIX_MATRIX);
// ...
}3.3 缓存策略问题位置: 问题:
建议:
4. 代码安全问题4.1 路径遍历漏洞位置: QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
if (!QFile::exists(originalPath)) {
return QStringList() << originalPath;
}
// ...
}问题:
建议: bool isValidFilePath(const QString &path) {
// 规范化路径
QString normalizedPath = QDir::cleanPath(path);
// 检查是否包含路径遍历
if (normalizedPath.contains("../") || normalizedPath.contains("..\\")) {
return false;
}
// 检查是否是绝对路径
QFileInfo fileInfo(normalizedPath);
if (!fileInfo.isAbsolute()) {
return false;
}
return true;
}
QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
if (!isValidFilePath(originalPath)) {
qWarning() << "Invalid file path:" << originalPath;
return QStringList();
}
if (!QFile::exists(originalPath)) {
return QStringList();
}
// ...
}4.2 权限问题位置: QString WallpaperCacheService::saveImageFromFd(const QDBusUnixFileDescriptor &fd, const QString &imagePathMd5)
{
int fdi = fd.fileDescriptor();
if (fdi <= 0) {
return QString();
}
QString originPath = kWallpaperCacheDir + "/" + imagePathMd5;
QFile originFile(originPath);
if (!originFile.open(QIODevice::ReadWrite)) {
qWarning() << "Failed to open destination file for writing:" << originPath;
return QString();
}
// ...
}问题:
建议:
4.3 资源耗尽攻击位置: void ScaleImageThread::addTasks(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
if (!QFile::exists(originalPath)) {
qWarning() << "file not exists:" << originalPath;
return;
}
QList<TaskData> tasks;
for (const QSize &size : sizes) {
TaskData data;
data.originalPath = originalPath;
data.targetSize = size;
data.isMd5Path = isMd5Path;
tasks.append(data);
}
QMutexLocker locker(&m_mutex);
for (const TaskData &task : tasks) {
if (!m_tasks.contains(task)) {
m_tasks.enqueue(task);
}
}
// ...
}问题:
建议: void ScaleImageThread::addTasks(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
if (!QFile::exists(originalPath)) {
qWarning() << "file not exists:" << originalPath;
return;
}
// 限制单个请求的尺寸数量
if (sizes.size() > 10) {
qWarning() << "Too many sizes requested:" << sizes.size();
return;
}
QList<TaskData> tasks;
for (const QSize &size : sizes) {
TaskData data;
data.originalPath = originalPath;
data.targetSize = size;
data.isMd5Path = isMd5Path;
tasks.append(data);
}
QMutexLocker locker(&m_mutex);
// 限制任务队列大小
if (m_tasks.size() > 100) {
qWarning() << "Task queue is full, rejecting new tasks";
return;
}
for (const TaskData &task : tasks) {
if (!m_tasks.contains(task)) {
m_tasks.enqueue(task);
}
}
// ...
}5. 其他建议5.1 添加日志和监控
5.2 添加单元测试
5.3 文档改进
总结该代码实现了一个壁纸缓存服务,主要功能包括图像缩放和模糊效果处理。代码整体结构清晰,但存在一些问题需要改进:
建议按照优先级逐步解决这些问题,特别是安全相关的问题应该优先处理。同时,建议添加更多的测试和文档,以提高代码的可靠性和可维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
The wallpapercache plugin provides the following main functions:
Log: implement wallpaper cache plugin for dde-services
Pms: TASK-384099