Skip to content
Merged
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
10 changes: 8 additions & 2 deletions ui/src/conversation-panel/main/chat-panel/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@ const handleDrop = (e: DragEvent) => input.addFiles(e.dataTransfer?.files)
<main class="mk-conversation-main" @drop.prevent="handleDrop" @dragover.prevent>
<!-- 头部可被替换(如 debug 用 debug header);默认普通 chat header -->
<slot name="header"><ChatHeader :shareable="shareable" @share="emit('share')" /></slot>
<MessageList :key="list.currentChatId.value" :app-name="list.appInfo.value?.name || 'AI 助手'"></MessageList>
<!-- 聊天背景铺满标题栏以下的消息区与输入区,与显示设置预览一致 -->
<div
class="flex-column min-h-0 flex-1 bg-cover bg-center"
:style="{ backgroundImage: list.appInfo.value?.chat_background ? `url(${JSON.stringify(list.appInfo.value.chat_background)})` : undefined }"
>
<MessageList :key="list.currentChatId.value" :app-name="list.appInfo.value?.name || 'AI 助手'"></MessageList>

<footer v-if="!list.isShared.value" class="mk-conversation-content flex-center shrink-0 p-4"><MessageInput /></footer>
<footer v-if="!list.isShared.value" class="mk-conversation-content flex-center shrink-0 p-4"><MessageInput /></footer>
</div>
</main>
</template>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here are the findings regarding irregularities, potential issues, and optimization suggestions for the provided code snippet:

1. Potential Issue: Unnecessary Double Quotes in URL Generation

Problem:
Using JSON.stringify() on the chat_background string wraps it in double quotes (e.g., "http://example.com/image.jpg"). When interpolated into a CSS url(), this results in:
background-image: url("http://example.com/image.jpg")

While many browsers handle this fine, it is redundant and potentially problematic if the underlying CSS parser is strict or if the string already contains escaped characters. It complicates potential future logic checking for specific protocols (like data:).

Suggestion:
Use the string directly without JSON.stringify. Ensure the value is sanitized if it comes from user input to prevent CSS injection/XSS, though for url(), risks are lower than in href.

// Instead of:
`url(${JSON.stringify(list.appInfo.value?.chat_background)})`

// Use:
`url(${list.appInfo.value.chat_background})`

2. Optimization: Conditional Style Object

Problem:
The inline style generates an object with undefined values when no background is set. While functional, creating a new object on every render can cause unnecessary re-renders or diffing overhead in some Vue configurations. A computed property or simpler ternary for the style object is cleaner.

Suggestion:
Extract the background style logic into a computed property for better readability and performance.

3. Security Warning: Missing Validation/Sanitization

Problem:
If list.appInfo.value.chat_background comes from user-configurable data (e.g., admin settings), inserting it directly into a CSS url() context carries a minor risk if the value is malformed (e.g., containing ) or breaking out of the url() context, though modern CSP helps mitigate this).

Suggestion:
Ensure chat_background is validated against allowed protocols (e.g., only https://, http://, or data:image/) before rendering. If using Vue's :style, ensure the content is trusted.

4. Layout/Rendering Note

Problem:
The comment says "background covers message area and input area," which is achieved by wrapping both in the div. This looks correct for the layout intent. However, if MessageList or <footer> have their own background colors (e.g., white), they will obscure the background image unless they are set to transparent (transparent or bg-transparent).

Check:
Verify that child components (<MessageList>, <MessageInput>) do not have opaque backgrounds defined in their scoped styles, or explicitly set them to transparent when a chat background is active.


Recommended Refactored Code

<script setup lang="ts">
import { computed } from 'vue';

// ... existing code

const chatBackgroundStyle = computed(() => {
  const bg = list.appInfo.value?.chat_background;
  if (!bg) return {};
  
  // Optional: Basic validation/sanitization here
  return {
    backgroundImage: `url("${bg}")`, 
    backgroundSize: 'cover',
    backgroundPosition: 'center',
  };
});
</script>

<template>
  <main class="mk-conversation-main" @drop.prevent="handleDrop" @dragover.prevent>
    <slot name="header">
      <ChatHeader :shareable="shareable" @share="emit('share')" />
    </slot>

    <!-- Apply background via computed style or direct interpolation -->
    <div
      class="flex-column min-h-0 flex-1"
      :style="chatBackgroundStyle"
      :class="{ 'bg-cover bg-center': !!list.appInfo.value?.chat_background }"
    >
      <MessageList 
        :key="list.currentChatId.value" 
        :app-name="list.appInfo.value?.name || 'AI 助手'" 
      />

      <footer v-if="!list.isShared.value" class="mk-conversation-content flex-center shrink-0 p-4">
        <MessageInput />
      </footer>
    </div>
  </main>
</template>

Note: Added quotes around ${bg} in url("${bg}") to safely handle spaces or special characters in filenames/URIs, which is best practice.

8 changes: 7 additions & 1 deletion ui/src/conversation-panel/main/display-panel/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ const list = useConversationListStore()
<!-- 只显示变体:仅 message-list,无 header、无 message-input(不发起对话)。
里面的 form 拿不到发送能力时自动只读。 -->
<main class="mk-conversation-main">
<MessageList :key="list.currentChatId.value" :app-name="list.appInfo.value?.name || 'AI 助手'" />
<!-- 聊天背景铺满消息区,与显示设置预览一致 -->
<div
class="flex-column min-h-0 flex-1 bg-cover bg-center"
:style="{ backgroundImage: list.appInfo.value?.chat_background ? `url(${JSON.stringify(list.appInfo.value.chat_background)})` : undefined }"
>
<MessageList :key="list.currentChatId.value" :app-name="list.appInfo.value?.name || 'AI 助手'" />
</div>
</main>
</template>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here is a review of the provided Vue code snippet, highlighting potential issues and optimization suggestions.

1. Critical Issue: JSON.stringify in CSS URL

Problem: You are using JSON.stringify(list.appInfo.value.chat_background) inside the url() function.

  • JSON.stringify("https://example.com/bg.jpg") produces "\"https://example.com/bg.jpg\"" (with double quotes inside the string).
  • This results in invalid CSS like: background-image: url(""https://example.com/bg.jpg""), which will fail to load the image.

Fix: Use the raw value directly or properly escape it without adding extra JSON quotes.

// Bad
`url(${JSON.stringify(list.appInfo.value.chat_background)})`

// Good
`url("${list.appInfo.value.chat_background}")`

2. Security Risk: XSS via Inline Style

Potential Issue: If chat_background comes from user-generated content or an untrusted API, injecting it directly into CSS url() can allow CSS Injection Attacks or Data URI exploits.

  • Mitigation: Ensure chat_background is validated (e.g., starts with http/https/ or is a safe base64 image) before rendering.

3. Performance & Reactivity

Observation: The :style binding depends on list.appInfo.value.chat_background.

  • If list is a reactive store (Pinia/Vuex), this is fine.
  • However, if appInfo changes frequently, inline styles can cause minor re-paints. For production, consider mapping this to a CSS class or using a dedicated background component if complex.

4. Layout & Flexbox Best Practices

Current Code:

<div class="flex-column min-h-0 flex-1 bg-cover bg-center">
  <MessageList :key="..." />
</div>

Suggestions:

  1. Overflow Handling: min-h-0 is correct for allowing flex children to shrink, but you likely need overflow-y: auto on this wrapper if MessageList doesn't handle its own scrolling. Verify that MessageList has its own scroll container; otherwise, long chats may break layout.
  2. Accessibility: The background div is purely decorative. Add aria-hidden="true" to prevent screen readers from announcing it unnecessarily (though modern ARIA rules usually ignore non-focusable divs with no text, it's safer):
    <div class="..." aria-hidden="true">

5. Code Cleanliness / Optimization

  • Computed Property: Computing the style object inline in the template can be slightly less performant than using a computed property, especially if dependencies are complex.
  • Conditional Logic: The ternary operator is verbose. Consider extracting it.

✅ Recommended Refactored Code

<script setup lang="ts">
import { computed } from 'vue';
import { useConversationListStore } from '@/stores'; // Adjust import path as needed

const list = useConversationListStore();

// Optional: Compute the style object separately for clarity and testability
const chatBackgroundStyle = computed(() => {
  const bg = list.appInfo?.value?.chat_background;
  if (!bg) return {};
  
  // Basic sanitization: Only allow http/https/data:image URLs to prevent JS/CSS injection
  if (/^(https?:\/\/|data:image\/)/i.test(bg)) {
    return { backgroundImage: `url("${bg}")` };
  }
  return {};
});

// Key stability check: currentChatId.value might be undefined initially
// Vue handles undefined keys gracefully, but ensure MessageList can handle key change resets
</script>

<template>
  <!-- ... previous context -->
  <main class="mk-conversation-main">
    <div
      class="flex-column min-h-0 flex-1 overflow-hidden bg-cover bg-center" 
      :style="chatBackgroundStyle"
      aria-hidden="true"
    >
      <MessageList 
        :key="list.currentChatId?.value || 'default-key'" 
        :app-name="list.appInfo?.value?.name || 'AI 助手'" 
      />
    </div>
  </main>
  <!-- ... -->
</template>

<style scoped>
/* Ensure the message list fills available space and scrolls internally */
.min-h-0 {
  min-height: 0px; /* Critical for flex shrinking */
}
.overflow-hidden {
  overflow: hidden; /* Prevent parent scroll if child handles it */
}
</style>

Key Changes Explained:

  1. Removed JSON.stringify and replaced with safe url("${bg}").
  2. Added regex check /^(https?:\/\/|data:image\/)/i to prevent malicious URI schemes (like javascript:...).
  3. Added overflow-hidden to the wrapper to ensure proper flex containment.
  4. Used a computed property for cleaner template code.

Loading