BUG: Wrong separator used in get_synonyms on Windows

[nymwo]

Describe the bug

The Synonym feature doesn't work when hosted on Windows.

In wp-content\plugins\elasticpress\includes\classes\Feature\Search\Synonyms.php we can se that the synonyms are separated (explode) by PHP_EOL, which is \r\n on Windows:

	public function get_synonyms() {
		$synonyms_raw = $this->get_synonyms_raw();
		$synonyms     = array_values(
			array_filter(
				array_map( [ $this, 'validate_synonym' ], explode( PHP_EOL, $synonyms_raw ) )
			)
		);

		/**
		 * Filter array of synonyms to add to a custom synonym filter.
		 *
		 * @hook ep_synonyms
		 * @return  {array} The new array of search synonyms.
		 */
		return apply_filters( 'ep_synonyms', $synonyms );
	}

But it looks like $this->get_synonyms_raw() always returns a string with synonyms separated by \n. On Linux, PHP_EOL is defined as \n, so this issue does not appear when running on that OS.

Changing PHP_EOL to "\n" resolves the issue on Windows.

Steps to Reproduce

1. Host on a Windows environment
2. Add synonyms
3. Try to use those synonyms

Screenshots, screen recording, code snippet

No response

Environment information

No response

WordPress and ElasticPress information

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

[felipeelia]

@nymwo is your database also hosted on Windows? The synonyms content comes from the post_content of a CPT called ep-synonym, and is saved by the handle_update_synonyms method. Can you check exactly where that \r\n becomes \n?

[github-actions]

It has been 3 days since more information was requested from you in this issue and we have not heard back. This issue is now marked as stale and will be closed in 3 days, but if you have more information to add then please comment and the issue will stay open.

[nymwo]

Yes, the database is on Windows.

This is what is sent when i press "Save changes" on the synonym admin page:

{"mode":"simple","solr":"# Defined synonyms.\n\nTest, Try\n\n# Defined hyponyms.\n\nblue => blue, aqua, azure, cerulean, cyan, ultramarine\n\n# Defined replacements.\n\nsupposably => supposedly\nflustrated => flustered, frustrated\nintensive purposes => intents and purposes\n"}

Though minified, I can see that synonyms-script.js joins the lines with "\n" when constructing the "solr" parameter. Changing that to use PHP_EOL might fix this.

[github-actions]

It has been 3 days since more information was requested from you in this issue and we have not heard back. This issue is now marked as stale and will be closed in 3 days, but if you have more information to add then please comment and the issue will stay open.

[felipeelia]

@nymwo do you want to craft a PR with that change? Thanks!

[nymwo]

I'm not sure where it makes sense to make the change. If not passing PHP_EOL to the JS script, maybe there should be a line break fix here:

ElasticPress/includes/classes/REST/Synonyms.php

Lines 94 to 118 in b432690

	public function update_synonyms( \WP_REST_Request $request ) {
	$feature = Features::factory()->get_registered_feature( 'search' )->synonyms;
	$mode = $request->get_param( 'mode' );
	$solr = $request->get_param( 'solr' );
	$post_id = $feature->update_synonym_post( $solr );
	if ( ! $post_id || is_wp_error( $post_id ) ) {
	return new \WP_Error( 'error-update-post' );
	}
	$updated = $feature->update_synonyms();
	if ( ! $updated ) {
	return new \WP_Error( 'error-update-index' );
	}
	$feature->save_editor_mode( $mode );
	return [
	'data' => $solr,
	'success' => true,
	];
	}

or here:

ElasticPress/includes/classes/Feature/Search/Synonyms.php

Lines 859 to 874 in b432690

	public function update_synonym_post( $content ) {
	$synonym_post_id = $this->get_synonym_post_id();
	if ( ! $synonym_post_id ) {
	return $synonym_post_id;
	}
	return wp_insert_post(
	[
	'ID' => $synonym_post_id,
	'post_content' => $content,
	'post_type' => self::POST_TYPE_NAME,
	],
	true
	);
	}

[felipeelia]

@nymwo I think the sanitize_solr method would be the best place to change that, so we have the input corrected as soon as possible in the flow. We are already wrapping up the 5.1.4 release and as we won't have time to test this change in this release, I'm punting this issue to 5.2.0, so you'll have some more time to craft that PR. Thanks in advance!