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 double-encoding issues #73

Merged
merged 5 commits into from
Jul 17, 2022
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [UNRELEASED]

### Fixed
- NovaEditorJsCast now properly handles JSON, not double-encoding stuff and decoding double-encoded properties.

## [3.0.3]

### Fixed
Expand Down
22 changes: 12 additions & 10 deletions src/NovaEditorJsCast.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class NovaEditorJsCast implements CastsAttributes
/**
* Magic number to describe "this field is broken" in the version
*/
private const BROKEN_VERSION = '2.12.9999';
public const BROKEN_VERSION = '2.12.9999';

/**
* Returns an instance of the EditorJsData field warning the user
Expand Down Expand Up @@ -47,14 +47,16 @@ private static function getErrorObject(string $exceptionMessage): NovaEditorJsDa
* @param array $attributes
* @return NovaEditorJsData|null
*/
public function get($model, string $key, $value, array $attributes)
public function get($model, string $key, $value, array $attributes): ?NovaEditorJsData
{
if ($value === null) {
return null;
}

try {
return new NovaEditorJsData(json_decode($value, true, 512, JSON_THROW_ON_ERROR));
// Recursively decode JSON, to solve a bug where the JSON is double-encoded.
while (is_string($value) && ! empty($value)) {
$value = json_decode($value, true, 512, JSON_THROW_ON_ERROR);
}

// Return null if the new value is null
return $value === null ? null : new NovaEditorJsData($value);
} catch (JsonException $exception) {
return self::getErrorObject($exception->getMessage());
}
Expand All @@ -67,9 +69,9 @@ public function get($model, string $key, $value, array $attributes)
* @param string $key
* @param mixed $value
* @param array $attributes
* @return mixed
* @return array
*/
public function set($model, string $key, $value, array $attributes)
public function set($model, string $key, $value, array $attributes): array
{
if ($value === null) {
return [
Expand All @@ -83,7 +85,7 @@ public function set($model, string $key, $value, array $attributes)
}

return [
$key => json_encode($value),
$key => is_string($value) ? $value : json_encode($value),
];
}
}
27 changes: 27 additions & 0 deletions tests/Fixtures/Models/Dummy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Tests\Fixtures\Models;

use Advoor\NovaEditorJs\NovaEditorJsCast;
use Advoor\NovaEditorJs\NovaEditorJsData;
use Illuminate\Database\Eloquent\Model;

/**
* @property NovaEditorJsData $data
* @method static \Illuminate\Database\Eloquent\Builder|Dummy newModelQuery()
* @method static \Illuminate\Database\Eloquent\Builder|Dummy newQuery()
* @method static \Illuminate\Database\Eloquent\Builder|Dummy query()
* @mixin \Eloquent
*/
class Dummy extends Model
{
protected $casts = [
'data' => NovaEditorJsCast::class,
];

protected $fillable = [
'data',
];
}
20 changes: 20 additions & 0 deletions tests/Fixtures/TestServiceProvider.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Tests\Fixtures;

use Illuminate\Support\ServiceProvider;

class TestServiceProvider extends ServiceProvider
{
/**
* Bootstrap any package services.
*
* @return void
*/
public function boot()
{
$this->app->useDatabasePath(__DIR__ . '/database/');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

declare(strict_types=1);

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

class CreateDummiesTable extends Migration
{
/**
* Run the migrations.
*
* @return void
*/
public function up()
{
Schema::create('dummies', function (Blueprint $table) {
$table->increments('id');
$table->json('data')->nullable();
$table->timestamps();
});
}

/**
* Reverse the migrations.
*
* @return void
*/
public function down()
{
Schema::drop('dummies');
}
}
12 changes: 11 additions & 1 deletion tests/TestCase.php
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
<?php

declare(strict_types=1);

namespace Tests;

use Advoor\NovaEditorJs\FieldServiceProvider;
use Illuminate\Foundation\Testing\DatabaseMigrations;
use Orchestra\Testbench\TestCase as OrchestraTestCase;
use Tests\Fixtures\TestServiceProvider;

/**
* Defines basic system to test with
*/
class TestCase extends OrchestraTestCase
{
use DatabaseMigrations;

/**
* Path to config file from here
*/
Expand All @@ -23,7 +29,10 @@ class TestCase extends OrchestraTestCase
*/
protected function getPackageProviders($app)
{
return [FieldServiceProvider::class];
return [
FieldServiceProvider::class,
TestServiceProvider::class,
];
}

/**
Expand All @@ -35,5 +44,6 @@ protected function getPackageProviders($app)
protected function getEnvironmentSetUp($app)
{
$app['config']->set('nova-editor-js', require self::CONFIG_PATH);
$app['config']->set('database.default', 'testing');
}
}
5 changes: 2 additions & 3 deletions tests/Unit/JsonContentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@

use Advoor\NovaEditorJs\NovaEditorJs;
use Tests\TestCase;
use Illuminate\Foundation\Testing\RefreshDatabase;

class JsonContentTest extends TestCase
{
/**
* Path to JSON file with valid contents
*/
private const TEST_FILE_JSON = __DIR__ . '/../resources/json/editorjs.json';
private const TEST_FILE_HTML = __DIR__ . '/../resources/html/editorjs.html';
private const TEST_FILE_JSON = __DIR__ . '/../Fixtures/resources/json/editorjs.json';
private const TEST_FILE_HTML = __DIR__ . '/../Fixtures/resources/html/editorjs.html';

/**
* Returns JSON contents from the file
Expand Down
111 changes: 111 additions & 0 deletions tests/Unit/NovaEditorJsCastTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
<?php

declare(strict_types=1);

namespace Tests\Unit;

use Advoor\NovaEditorJs\NovaEditorJsCast;
use Advoor\NovaEditorJs\NovaEditorJsData;
use Tests\TestCase;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\DB;
use Tests\Fixtures\Models\Dummy;

class NovaEditorJsCastTest extends TestCase
{
use RefreshDatabase;

private const VALID_BLOCK = [
'time' => 1658064476,
'blocks' => [
[
'type' => 'paragraph',
'data' => [
'text' => 'Paragraph'
]
]
],
'version' => '2.3.0',
];

/**
* Test a very basic save-and-decode.
*/
public function testSavingAndDecoding(): void
{
Dummy::create(['data' => self::VALID_BLOCK]);

$instance = Dummy::first();

$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
}

/**
* Test saving and decoding a value that's already JSON-encoded.
*/
public function testSavingPreCompiledJson(): void
{
Dummy::create(['data' => json_encode(self::VALID_BLOCK)]);

$instance = Dummy::first();

$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
}

/**
* Test decoding a value that's double-JSON-encoded, basically bug mitigation.
*/
public function testReadingDoubleEncodedJson(): void
{
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?)', [json_encode(json_encode(self::VALID_BLOCK))]);

$instance = Dummy::first();

$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
}

/**
* Test reading null values in JSON.
*/
public function testReadingNullValues(): void
{
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?), (null)', [json_encode(null)]);

[$jsonInstance, $nullInstance] = Dummy::get();

$this->assertNull($jsonInstance->data);
$this->assertNull($nullInstance->data);
}

/**
* Test writing null values, a json-encoded null value
* should be stored as-is.
*/
public function testReadingNullValue(): void
{
Dummy::create(['data' => null]);
Dummy::create(['data' => json_encode(null)]);

$rows = DB::select('SELECT `id`, `data` FROM `dummies`');

$this->assertNull($rows[0]->data);
$this->assertSame('null', $rows[1]->data);
}

/**
* Finally, check if reading broken JSON is handled properly.
*/
public function testReadingInvalidJson(): void
{
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?)', ['{"}']);

$instance = Dummy::first();

$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
$this->assertNotNull($instance->data->version, 'Expected version key on data');
$this->assertEquals(NovaEditorJsCast::BROKEN_VERSION, $instance->data->version, 'Expected version to match "broken" version');
}
}