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

Add support for UTF-8 feature extension. #722

Merged
merged 14 commits into from
Jun 22, 2018
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 4 additions & 0 deletions src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ final class TDS {
static final int AES_256_CBC = 1;
static final int AEAD_AES_256_CBC_HMAC_SHA256 = 2;
static final int AE_METADATA = 0x08;

static final byte TDS_FEATURE_EXT_UTF8SUPPORT = 0x0A;

static final int TDS_TVP = 0xF3;
static final int TVP_ROW = 0x01;
Expand Down Expand Up @@ -177,6 +179,8 @@ static final String getTokenName(int tdsTokenType) {
return "TDS_DONEINPROC (0xFF)";
case TDS_FEDAUTHINFO:
return "TDS_FEDAUTHINFO (0xEE)";
case TDS_FEATURE_EXT_UTF8SUPPORT:
return "TDS_FEATURE_EXT_UTF8SUPPORT (0x0A)";
default:
return "unknown token (0x" + Integer.toHexString(tdsTokenType).toUpperCase() + ")";
}
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLCollation.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ final class SQLCollation implements java.io.Serializable
private int langID() { return info & 0x0000FFFF; }
private final int sortId; // 5th byte of TDS collation.
private final Encoding encoding;
private static final int UTF8_IN_TDSCOLLATION = 0x4000000;

// Utility methods for getting details of this collation's encoding
final Charset getCharset() throws SQLServerException { return encoding.charset(); }
Expand Down Expand Up @@ -77,8 +78,13 @@ int getCollationSortID() {
*/
info = tdsReader.readInt(); // 4 bytes, contains: LCID ColFlags Version
sortId = tdsReader.readUnsignedByte(); // 1 byte, contains: SortId
// For a SortId==0 collation, the LCID bits correspond to a LocaleId
encoding = (0 == sortId) ? encodingFromLCID() : encodingFromSortId();
if (UTF8_IN_TDSCOLLATION == (info & UTF8_IN_TDSCOLLATION)) {
encoding = Encoding.UTF8;
}
else {
// For a SortId==0 collation, the LCID bits correspond to a LocaleId
encoding = (0 == sortId) ? encodingFromLCID() : encodingFromSortId();
}
}

/**
Expand Down Expand Up @@ -549,6 +555,7 @@ private Encoding encodingFromSortId() throws UnsupportedEncodingException {
enum Encoding
{
UNICODE ("UTF-16LE", true, false),
UTF8 ("UTF-8", true, false),
CP437 ("Cp437", false, false),
CP850 ("Cp850", false, false),
CP874 ("MS874", true, true),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3432,6 +3432,16 @@ int writeFedAuthFeatureRequest(boolean write,
return totalLen;
}

int writeUTF8SupportFeatureRequest(boolean write,
TDSWriter tdsWriter /* if false just calculates the length */) throws SQLServerException {
int len = 5; // 1byte = featureID, 4bytes = featureData length
if (write) {
tdsWriter.writeByte(TDS.TDS_FEATURE_EXT_UTF8SUPPORT);
tdsWriter.writeInt(0);
}
return len;
}

private final class LogonCommand extends UninterruptableTDSCommand {
LogonCommand() {
super("logon");
Expand Down Expand Up @@ -4130,7 +4140,19 @@ private void onFeatureExtAck(int featureId,
serverSupportsColumnEncryption = true;
break;
}
case TDS.TDS_FEATURE_EXT_UTF8SUPPORT: {
if (connectionlogger.isLoggable(Level.FINER)) {
connectionlogger.fine(toString() + " Received feature extension acknowledgement for UTF8 support.");
}

if (1 > data.length) {
if (connectionlogger.isLoggable(Level.SEVERE)) {
connectionlogger.severe(toString() + " Unknown value for UTF8 support.");
Copy link
Contributor

Choose a reason for hiding this comment

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

use R_unknownUTF8SupportValue here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, we don't localize the logger messages.

}
throw new SQLServerException(SQLServerException.getErrString("R_unknownUTF8SupportValue"), null);
}
break;
}
default: {
// Unknown feature ack
if (connectionlogger.isLoggable(Level.SEVERE)) {
Expand Down Expand Up @@ -4419,6 +4441,8 @@ else if (serverMajorVersion >= 9) // Yukon (9.0) --> TDS 7.2 // Prelogin disconn

len2 = len2 + 1; // add 1 to length becaue of FeatureEx terminator

len2 = len2 + writeUTF8SupportFeatureRequest(false, tdsWriter);
Copy link
Member

Choose a reason for hiding this comment

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

This is a little convoluted, the function basically always returns "5". It also has a Boolean to turn on/off the actual writing. Shouldn't we remove the length calculation from this function and just have a static variable which equals 5? Can also remove the Boolean parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done to keep the same structure for all feature extension requests, in case we calculate the length differently in the future. Take a look at writeFedAuthFeatureRequest, writeAEFeatureRequest, writeDataClassificationFeatureRequest, The methods are implemented the same way in .NET driver too.

Copy link
Member

Choose a reason for hiding this comment

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

It makes sense with fedAuth because the length is actually variable. For the others, it seems there's a set value(6 for AE, 5 for UTF8, 6 for data classification). I guess it's fine if we're consistent, but I'm just saying that other than the fedAuth function, the others don't really make a lot of sense.


// Length of entire Login 7 packet
tdsWriter.writeInt(len2);
tdsWriter.writeInt(tdsVersion);
Expand Down Expand Up @@ -4598,6 +4622,8 @@ else if (serverMajorVersion >= 9) // Yukon (9.0) --> TDS 7.2 // Prelogin disconn
writeFedAuthFeatureRequest(true, tdsWriter, fedAuthFeatureExtensionData);
}

writeUTF8SupportFeatureRequest(true, tdsWriter);
Copy link
Member

Choose a reason for hiding this comment

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

Length is being "calculated" and returned, but we don't use the value. Seems like we're always using half of the functionalities.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes I've never liked this. It's not really "calculated" at all the whole thing is just hardcoded


tdsWriter.writeByte((byte) TDS.FEATURE_EXT_TERMINATOR);
tdsWriter.setDataLoggable(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,5 +393,6 @@ protected Object[][] getContents() {
{"R_invalidSSLProtocol", "SSL Protocol {0} label is not valid. Only TLS, TLSv1, TLSv1.1, and TLSv1.2 are supported."},
{"R_cancelQueryTimeoutPropertyDescription", "The number of seconds to wait to cancel sending a query timeout."},
{"R_invalidCancelQueryTimeout", "The cancel timeout value {0} is not valid."},
{"R_unknownUTF8SupportValue", "Unknown value for UTF8 support."},
};
}
}
159 changes: 159 additions & 0 deletions src/test/java/com/microsoft/sqlserver/jdbc/unit/UTF8SupportTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package com.microsoft.sqlserver.jdbc.unit;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.nio.charset.StandardCharsets;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Collections;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.platform.runner.JUnitPlatform;
import org.junit.runner.RunWith;

import com.microsoft.sqlserver.testframework.AbstractSQLGenerator;
import com.microsoft.sqlserver.testframework.AbstractTest;
import com.microsoft.sqlserver.testframework.PrepUtil;
import com.microsoft.sqlserver.testframework.Utils;
import com.microsoft.sqlserver.testframework.util.RandomUtil;

/**
* A class for testing the UTF8 support changes.
*/
@RunWith(JUnitPlatform.class)
public class UTF8SupportTest extends AbstractTest {
private static Connection connection;
private static String databaseName;
private static String tableName;

/**
* Test against UTF8 CHAR type.
*
* @throws SQLException
*/
@Test
public void testChar() throws SQLException {
createTable("char(10)");
validate("teststring");
// This is 10 UTF-8 bytes. D1 82 D0 B5 D1 81 D1 82 31 32
validate("тест12");
// E2 95 A1 E2 95 A4 E2 88 9E 2D
validate("╡╤∞-");

createTable("char(4000)");
validate(String.join("", Collections.nCopies(400, "teststring")));
validate(String.join("", Collections.nCopies(400, "тест12")));
validate(String.join("", Collections.nCopies(400, "╡╤∞-")));

createTable("char(4001)");
validate(String.join("", Collections.nCopies(400, "teststring")) + "1");
validate(String.join("", Collections.nCopies(400, "тест12")) + "1");
validate(String.join("", Collections.nCopies(400, "╡╤∞-")) + "1");

createTable("char(8000)");
validate(String.join("", Collections.nCopies(800, "teststring")));
validate(String.join("", Collections.nCopies(800, "тест12")));
validate(String.join("", Collections.nCopies(800, "╡╤∞-")));
}

/**
* Test against UTF8 VARCHAR type.
*
* @throws SQLException
*/
@Test
public void testVarchar() throws SQLException {
createTable("varchar(10)");
validate("teststring");
validate("тест12");
validate("╡╤∞-");

createTable("varchar(4000)");
validate(String.join("", Collections.nCopies(400, "teststring")));
validate(String.join("", Collections.nCopies(400, "тест12")));
validate(String.join("", Collections.nCopies(400, "╡╤∞-")));

createTable("varchar(4001)");
validate(String.join("", Collections.nCopies(400, "teststring")) + "1");
validate(String.join("", Collections.nCopies(400, "тест12")) + "1");
validate(String.join("", Collections.nCopies(400, "╡╤∞-")) + "1");

createTable("varchar(8000)");
validate(String.join("", Collections.nCopies(800, "teststring")));
validate(String.join("", Collections.nCopies(800, "тест12")));
validate(String.join("", Collections.nCopies(800, "╡╤∞-")));

createTable("varchar(MAX)");
validate(String.join("", Collections.nCopies(800, "teststring")));
validate(String.join("", Collections.nCopies(800, "тест12")));
validate(String.join("", Collections.nCopies(800, "╡╤∞-")));
}

@BeforeAll
public static void setUp() throws ClassNotFoundException, SQLException {
databaseName = RandomUtil.getIdentifier("UTF8Database");
tableName = AbstractSQLGenerator.escapeIdentifier(RandomUtil.getIdentifier("RequestBoundaryTable"));
connection = PrepUtil.getConnection(getConfiguredProperty("mssql_jdbc_test_connection_properties"));
Copy link
Member

Choose a reason for hiding this comment

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

who closes this connection?

createDatabaseWithUTF8Collation();
connection.setCatalog(databaseName);
}

@AfterAll
public static void cleanUp() throws SQLException {
Utils.dropDatabaseIfExists(databaseName, connection.createStatement());
}

private static void createDatabaseWithUTF8Collation() throws SQLException {
try (Statement stmt = connection.createStatement();) {
stmt.executeUpdate("CREATE DATABASE " + AbstractSQLGenerator.escapeIdentifier(databaseName) + " COLLATE Cyrillic_General_100_CS_AS_UTF8");
}
}

private static void createTable(String columnType) throws SQLException {
try (Statement stmt = connection.createStatement();) {
Utils.dropTableIfExists(tableName, stmt);
stmt.executeUpdate("CREATE TABLE " + tableName + " (c " + columnType + ")");
}
}

public void clearTable() throws SQLException {
try (Statement stmt = connection.createStatement();) {
stmt.executeUpdate("DELETE FROM " + tableName);
}
}

public void validate(String value) throws SQLException {
if (Utils.serverSupportsUTF8(connection)) {
try (PreparedStatement psInsert = connection.prepareStatement("INSERT INTO " + tableName + " VALUES(?)");
PreparedStatement psFetch = connection.prepareStatement("SELECT * FROM " + tableName);
Statement stmt = connection.createStatement();) {
clearTable();
// Used for exact byte comparison.
byte[] valueBytes = value.getBytes(StandardCharsets.UTF_8);

psInsert.setString(1, value);
psInsert.executeUpdate();

// Fetch using Statement.
ResultSet rsStatement = stmt.executeQuery("SELECT * FROM " + tableName);
rsStatement.next();
// Compare Strings.
assertEquals(value, rsStatement.getString(1));
// Test UTF8 sequence returned from getBytes().
assertArrayEquals(valueBytes, rsStatement.getBytes(1));

// Fetch using PreparedStatement.
ResultSet rsPreparedStatement = psFetch.executeQuery();
rsPreparedStatement.next();
assertEquals(value, rsPreparedStatement.getString(1));
assertArrayEquals(valueBytes, rsPreparedStatement.getBytes(1));
}
}
}
}
11 changes: 10 additions & 1 deletion src/test/java/com/microsoft/sqlserver/testframework/Utils.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
import java.io.CharArrayReader;
import java.net.URI;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.logging.Level;
Expand Down Expand Up @@ -286,7 +288,7 @@ public static void dropProcedureIfExists(String procName, java.sql.Statement stm

public static void dropDatabaseIfExists(String databaseName,
java.sql.Statement stmt) throws SQLException {
stmt.executeUpdate("IF EXISTS(SELECT * from sys.databases WHERE name='" + databaseName + "') DROP DATABASE [" + databaseName + "]");
stmt.executeUpdate("USE MASTER; IF EXISTS(SELECT * from sys.databases WHERE name='" + databaseName + "') DROP DATABASE [" + databaseName + "]");
}

/**
Expand Down Expand Up @@ -328,4 +330,11 @@ public static boolean isJDBC43OrGreater(Connection connection) throws SQLExcepti
public static float getJDBCVersion(Connection connection) throws SQLException {
return Float.valueOf(connection.getMetaData().getJDBCMajorVersion() + "." + connection.getMetaData().getJDBCMinorVersion());
}

public static boolean serverSupportsUTF8(Connection connection) throws SQLException {
try (Statement stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery("SELECT name FROM sys.fn_helpcollations() WHERE name LIKE '%UTF8%'");) {
return rs.isBeforeFirst();
}
}
}