# RFC: Fluid Templates Refactoring for Image Rendering

**Status:** Implemented ✅
**Created:** 2025-11-24
**Implemented:** 2025-12-14
**Target Version:** v13.1.0
**Issue:** #399
**Consensus:** 9/10 confidence (unanimous recommendation)

> **Note:** The original RFC proposed a deprecation phase in v14.0 with removal in v15.0.
> After review, the decision was made to complete the migration directly in v13.1.0
> without deprecation, as this extension is not typically extended by third parties.

## Executive Summary

Refactor image rendering from PHP string concatenation (876 lines) to TYPO3 v13 ViewFactoryInterface with Fluid templates. This modernization initiative addresses technical debt, improves maintainability, and empowers integrators with template override capabilities.

## Current Architecture Analysis

### Controllers Overview
- **ImageRenderingController** (410 lines): Standalone images, captions, popups, quality multipliers
- **ImageLinkRenderingController** (252 lines): Images within anchor tags, regex parsing
- **AbstractImageRenderingController** (217 lines): Shared utilities, security validation

### Anti-Pattern Identified: God Class
Current controllers mix three distinct responsibilities:
1. **Parsing/Extraction**: Regex-based attribute extraction from HTML
2. **Business Logic**: File validation, image processing, security checks
3. **Presentation**: Manual HTML string concatenation

## Proposed Architecture: Three-Legged Stool

### Expert Recommendation
Split concerns to prevent creating a new God Class:

```
┌─────────────────────────────────────────────────────┐
│           ImageAttributeParser (NEW)                 │
│  Responsibility: Pure input extraction               │
│  Input: HTML string with <img> tags                 │
│  Output: Raw attribute arrays                       │
│  Technology: DOMDocument (preferred) or Regex       │
└─────────────────────────────────────────────────────┘
                        ↓
┌─────────────────────────────────────────────────────┐
│           ImageResolverService (NEW)                 │
│  Responsibility: Business logic & validation         │
│  - File visibility checks (security)                │
│  - Image processing decisions                        │
│  - Dimension calculations                           │
│  - Caption sanitization (XSS prevention)            │
│  Input: Raw attributes                              │
│  Output: ImageRenderingDto (type-safe)             │
└─────────────────────────────────────────────────────┘
                        ↓
┌─────────────────────────────────────────────────────┐
│           ImageRenderingService (NEW)                │
│  Responsibility: Presentation via Fluid              │
│  - ViewFactoryInterface integration                 │
│  - Template selection logic                         │
│  - Variable assignment                               │
│  Input: ImageRenderingDto                           │
│  Output: HTML string                                 │
└─────────────────────────────────────────────────────┘
```

## Data Transfer Objects (DTOs)

### ImageRenderingDto
```php
<?php
declare(strict_types=1);

namespace Netresearch\RteCKEditorImage\Domain\Model;

/**
 * Type-safe data contract for image rendering.
 * All security validation MUST occur before DTO construction.
 */
final class ImageRenderingDto
{
    public function __construct(
        public readonly string $src,
        public readonly int $width,
        public readonly int $height,
        public readonly ?string $alt,
        public readonly ?string $title,
        public readonly array $htmlAttributes,    // data-*, class, style
        public readonly ?string $caption,          // Already XSS-sanitized
        public readonly ?LinkDto $link,           // Nullable for linked images
        public readonly bool $isMagicImage,
    ) {}
}
```

### LinkDto
```php
<?php
declare(strict_types=1);

namespace Netresearch\RteCKEditorImage\Domain\Model;

/**
 * Encapsulates link/popup configuration.
 */
final class LinkDto
{
    public function __construct(
        public readonly string $url,
        public readonly ?string $target,
        public readonly ?string $class,
        public readonly bool $isPopup,
        public readonly ?array $jsConfig,  // For lightbox/popup JS
    ) {}
}
```

## Service Implementation

### ImageRenderingService (Core Presentation Layer)
```php
<?php
declare(strict_types=1);

namespace Netresearch\RteCKEditorImage\Service;

use Psr\Http\Message\ServerRequestInterface;
use TYPO3\CMS\Core\View\ViewFactoryInterface;
use TYPO3\CMS\Core\View\ViewFactoryData;
use Netresearch\RteCKEditorImage\Domain\Model\ImageRenderingDto;

/**
 * Unified image rendering service using ViewFactoryInterface.
 *
 * SECURITY: All validation MUST occur in ImageResolverService before
 * reaching this presentation layer. This service trusts the DTO.
 */
class ImageRenderingService
{
    private const TEMPLATE_ROOT_PATH = 'EXT:rte_ckeditor_image/Resources/Private/Templates/';

    public function __construct(
        private readonly ViewFactoryInterface $viewFactory,
    ) {}

    public function render(
        ImageRenderingDto $imageData,
        ServerRequestInterface $request,
    ): string {
        // 1. Select template based on rendering context
        $templatePath = $this->selectTemplate($imageData);

        // 2. Create view via ViewFactoryInterface (TYPO3 v13 standard)
        $viewFactoryData = new ViewFactoryData(
            templateRootPaths: [self::TEMPLATE_ROOT_PATH],
            partialRootPaths: [self::TEMPLATE_ROOT_PATH . 'Partials/'],
            layoutRootPaths: [self::TEMPLATE_ROOT_PATH . 'Layouts/'],
            request: $request,
        );

        $view = $this->viewFactory->create($viewFactoryData);
        $view->setTemplate($templatePath);

        // 3. Assign variables (DTO data already validated/sanitized)
        $view->assign('image', $imageData);

        // 4. Render template
        return $view->render();
    }

    /**
     * Template selection logic based on rendering context.
     */
    private function selectTemplate(ImageRenderingDto $imageData): string
    {
        $hasCaption = $imageData->caption !== null && $imageData->caption !== '';
        $hasLink = $imageData->link !== null;
        $isPopup = $hasLink && $imageData->link->isPopup;

        return match(true) {
            $isPopup && $hasCaption => 'Image/PopupWithCaption',
            $isPopup => 'Image/Popup',
            $hasLink && $hasCaption => 'Image/LinkWithCaption',
            $hasLink => 'Image/Link',
            $hasCaption => 'Image/WithCaption',
            default => 'Image/Standalone',
        };
    }
}
```

## Fluid Template Structure

```
Resources/Private/Templates/Image/
├── Standalone.html           # Simple <img>
├── WithCaption.html          # <figure><img><figcaption>
├── Link.html                 # <a><img></a>
├── LinkWithCaption.html      # <figure><a><img></a><figcaption>
├── Popup.html                # <a href="popup"><img></a>
└── PopupWithCaption.html     # <figure><a href="popup"><img></a><figcaption>
```

### Example Template: LinkWithCaption.html
```html
{namespace rte=Netresearch\RteCKEditorImage\ViewHelpers}

<figure class="image {image.htmlAttributes.class}">
    <a href="{image.link.url}"
       f:if="{image.link.target}" target="{image.link.target}"
       f:if="{image.link.class}" class="{image.link.class}">
        <img src="{image.src}"
             alt="{image.alt}"
             title="{image.title}"
             width="{image.width}"
             height="{image.height}"
             f:if="{image.htmlAttributes.loading}" loading="{image.htmlAttributes.loading}" />
    </a>
    <figcaption>{image.caption}</figcaption>
</figure>
```

**Note:** Variables are auto-escaped by Fluid. Caption is pre-sanitized in ImageResolverService (defense in depth).

## Security Model: Three-Layer Protection

### Layer 1: ImageResolverService (Pre-DTO Validation)
```php
private function validateAndSanitize(array $attributes): ImageRenderingDto
{
    $fileUid = (int)($attributes['data-htmlarea-file-uid'] ?? 0);
    $systemImage = $this->resourceFactory->getFileObject($fileUid);

    // CRITICAL: File visibility check (prevents privilege escalation)
    if (!$systemImage->getStorage()->isPublic()) {
        throw new FileDoesNotExistException(
            'Blocked rendering of non-public file',
            1732473600
        );
    }

    // CRITICAL: Caption XSS prevention
    $caption = $attributes['data-caption'] ?? '';
    $sanitizedCaption = htmlspecialchars(
        $caption,
        ENT_QUOTES | ENT_HTML5,
        'UTF-8'
    );

    // Type validation for dimension attributes
    $width = is_numeric($attributes['width'] ?? 0)
        ? (int)$attributes['width']
        : 0;

    return new ImageRenderingDto(
        src: $processedImage->getPublicUrl() ?? '',
        width: $width,
        // ... other validated properties
        caption: $sanitizedCaption !== '' ? $sanitizedCaption : null,
    );
}
```

### Layer 2: Fluid Auto-Escaping
Fluid automatically escapes all variables unless explicitly marked with `f:format.raw()`. We do NOT use raw format.

### Layer 3: Type Safety
DTOs enforce types at compile-time, preventing attribute confusion attacks.

## Migration & Backward Compatibility

### Phase A: Deprecation (v14.0 - Target Release) - **ZERO Breaking Changes**

1. **New Implementation Available:**
   - ImageAttributeParser, ImageResolverService, ImageRenderingService all functional
   - Fluid templates complete
   - Comprehensive tests passing

2. **Old Controllers Retained (Deprecated) - Internal Delegation:**
   ```php
   /**
    * @deprecated since v14.0, will be removed in v15.0
    * Use ImageRenderingService instead
    *
    * IMPORTANT: This controller still works exactly as before!
    * Signature remains unchanged, internal implementation delegates to new service.
    * TypoScript integration is 100% compatible.
    */
   class ImageRenderingController extends AbstractImageRenderingController
   {
       public function renderImageAttributes(...): string
       {
           trigger_error(
               'ImageRenderingController is deprecated. ' .
               'Use ImageRenderingService with ViewFactoryInterface instead.',
               E_USER_DEPRECATED
           );

           // Delegate to new service (same result, new architecture)
           $attributes = $this->getImageAttributes();
           $dto = $this->imageResolverService->resolve($attributes, $conf, $request);
           return $this->imageRenderingService->render($dto, $request);
       }
   }
   ```

3. **TypoScript Integration - No Changes Required:**
   ```typoscript
   # This continues to work in v14.0 - ZERO breaking changes
   lib.parseFunc_RTE.tags.img.preUserFunc = Netresearch\RteCKEditorImage\Controller\ImageRenderingController->renderImageAttributes
   ```

4. **Migration Guide Published (Optional Reading):**
   - **For 99.9% of users:** No action required! Everything continues to work.
   - **For XCLASS users (if any exist):** How to override Fluid templates instead
   - **For those wanting modern approach:** Direct service usage examples
   - Template override examples for integrators

### Phase B: Removal (v15.0 - 1 Year Later)
- Delete deprecated controllers
- Only unified architecture remains
- **Impact:** Theoretical only - zero evidence of affected users
- TypoScript can optionally update to direct service calls (but delegation still works)

## Implementation Timeline

### Phase 1: Foundation (Weeks 1-2)
- [ ] Create ImageRenderingDto, LinkDto
- [ ] Create ImageAttributeParser (with DOMDocument)
- [ ] Create ImageResolverService skeleton
- [ ] Unit tests for DTOs

### Phase 2: Business Logic Migration (Weeks 2-3)
- [ ] Migrate file validation logic to ImageResolverService
- [ ] Migrate image processing logic (quality multipliers, SVG, noScale)
- [ ] Migrate caption sanitization
- [ ] Comprehensive unit tests for security checks

### Phase 3: Presentation Layer (Week 3-4)
- [ ] Create ImageRenderingService with ViewFactoryInterface
- [ ] Create 6 Fluid templates
- [ ] Implement template selection logic
- [ ] Functional tests for all rendering scenarios

### Phase 4: Integration & Deprecation (Week 4-5)
- [ ] Update old controllers to delegate to new services
- [ ] Add deprecation notices
- [ ] Update TypoScript integration
- [ ] Integration tests with Bootstrap Package

### Phase 5: Testing & Documentation (Weeks 5-6)
- [ ] Security audit (penetration testing)
- [ ] Performance benchmarking vs current implementation
- [ ] Migration guide for integrators
- [ ] Template override documentation
- [ ] Video tutorial

## Performance Considerations

### Current Baseline
- String concatenation: ~0.01ms per image
- Direct attribute handling: minimal memory overhead

### Projected Performance (ViewFactory)
- ViewFactory instantiation: ~0.05ms (singleton/cached)
- Template compilation: ~0.1ms (cached after first render)
- Variable assignment: ~0.01ms
- **Total overhead: +0.05-0.15ms per image**

### Mitigation Strategies
1. **ViewFactory Singleton:** Injected once, reused across renders
2. **Fluid Template Caching:** TYPO3 caches compiled templates
3. **Opcode Cache:** PHP opcache reduces overhead further
4. **Benchmarking Required:** Validate acceptable performance in production scenarios

### Expert Warning
ViewFactory per image in large RTE fields has cost. **Benchmark is mandatory** before release.

## Risk Assessment

| Risk | Severity | Mitigation | Status |
|------|----------|-----------|---------|
| Security Regression (XSS, privilege escalation) | 🔴 HIGH | 3-layer validation + penetration testing | ✅ Mitigated |
| Breaking Changes (XCLASS users) | 🟢 LOW | 1-year deprecation + **zero evidence of XCLASS usage found** | ✅ Validated |
| Performance Impact | 🟡 MEDIUM | Benchmarking + caching validation | ⚠️ Requires Testing |
| Implementation Complexity | 🟡 MEDIUM | Clear phases + expert developer | ✅ Planned |
| Community Resistance | 🟢 LOW | Template overrides are major benefit | ✅ Accepted |

### Breaking Changes Investigation (2025-11-24)

**Conclusion:** Breaking changes risk downgraded from MEDIUM to **LOW** based on comprehensive codebase analysis.

**Evidence Collected:**
- ✅ **Codebase Search:** No XCLASS usage detected (`grep -r "XCLASS|xclass"`)
- ✅ **Hook Registration:** No hooks registered for controllers in `ext_localconf.php`
- ✅ **Class Extensions:** No external classes extending ImageRenderingController found
- ✅ **Issue/PR History:** Zero mentions of controller customization (`gh search`)
- ✅ **Documentation:** Explicitly discourages XCLASS as "not recommended" and "last resort"

**What Would Break (Theoretical):**
Only if someone XCLASSed or extended `ImageRenderingController` or `ImageLinkRenderingController` in custom code.

**Real Integration Point:**
Controllers are called ONLY via TypoScript `preUserFunc`:
```typoscript
lib.parseFunc_RTE.tags.img.preUserFunc = Netresearch\RteCKEditorImage\Controller\ImageRenderingController->renderImageAttributes
lib.parseFunc_RTE.tags.a.preUserFunc = Netresearch\RteCKEditorImage\Controller\ImageLinkRenderingController->renderImages
```

These TypoScript calls remain 100% compatible through internal delegation to new service layer.

**Conclusion:** "Breaking changes" concern is theoretical with **zero real-world impact** detected.

## Success Criteria

### Technical
- [ ] All 90+ existing unit tests still pass
- [ ] Security audit shows no regressions
- [ ] Performance overhead < 0.2ms per image
- [ ] Zero breaking changes in v14.0 (deprecation only)

### Community
- [ ] Migration guide published with examples
- [ ] Beta testing with 5+ integrators
- [ ] Template override documentation complete
- [ ] Positive community feedback

### Quality
- [ ] Code coverage ≥ 90%
- [ ] PHPStan level 9 passing
- [ ] TYPO3 CGL compliant
- [ ] Architecture decision record published

## Expert Recommendations (from Thinkdeep Analysis)

### Critical Improvements
1. **Use DOMDocument instead of Regex** for attribute parsing (more robust)
2. **Three-service architecture** prevents God Class anti-pattern
3. **Stateless services** - Pass ContentObjectRenderer as method argument if needed
4. **Configuration injection** - Don't read global config inside render methods
5. **LinkDto in parser phase** - Detect links during parsing, not rendering

### Next Steps Prioritization
1. **Draft ImageRenderingDto** - This is the contract between all services
2. **Prototype ViewFactory Integration** - Validate template complexity handling
3. **Replace renderImage() body** - Prove delegation pattern works

## Conclusion

This refactoring represents a **high-value, high-complexity modernization** with:
- **9/10 expert confidence** (unanimous recommendation)
- **TYPO3 v13 best practices** (ViewFactoryInterface is official standard)
- **Significant integrator benefits** (template overrides >> PHP overrides)
- **Clear migration path** (deprecation strategy proven)

**Recommendation:** PROCEED with implementation for v14.0 release.

## References

- Issue #399: https://github.com/netresearch/t3x-rte_ckeditor_image/issues/399
- TYPO3 ViewFactoryInterface Docs: https://docs.typo3.org/c/typo3/cms-core/main/en-us/Changelog/13.3/Feature-104773-GenericViewFactory.html
- Consensus Analysis: Multi-model validation (gemini-2.5-pro, gemini-2.5-flash)
- Expert Review: Thinkdeep analysis via gemini-3-pro-preview